* Re: How to find a commit that introduces (not removes) a string?
From: Sebastian Schuberth @ 2011-11-04 10:59 UTC (permalink / raw)
Cc: git, nkreitzinger, laksvij
In-Reply-To: <j8to8h$vqd$1@dough.gmane.org>
On 03.11.2011 10:50, Sebastian Schuberth wrote:
> I know about git log's -S / -G, but I'm unable to make these search through *introduced* strings only. Is there a way to do so?
Thanks for your suggestions. However, I ended up simply doing
$ git diff --no-color FROM..TO | grep ^+[^+] | grep WORD
which works well for my case.
--
Sebastian Schuberth
^ permalink raw reply
* important upgrade info for Fedora/EPEL RPM of gitolite
From: Sitaram Chamarty @ 2011-11-04 13:57 UTC (permalink / raw)
To: gitolite, git; +Cc: limb
[-- Attachment #1: Type: text/plain, Size: 843 bytes --]
Hello,
If you're using a Fedora/EPEL RPM of gitolite, and
upgraded/upgrading from 1.5.* to 2.*, then please do this as
soon as possible after the upgrade.
(1) look in ~/.gitolite.rc on the server for these variables:
$GL_PACKAGE_CONF
$GL_PACKAGE_HOOKS
If they are both defined, (and uncommented), you should be ok.
(2) if they are not defined, please add these lines at or near
the top of the file:
$GL_PACKAGE_CONF="/usr/share/gitolite/conf";
$GL_PACKAGE_HOOKS="/usr/share/gitolite/hooks";
Save the file, then run gl-setup (without any arguments).
This will fix up some issues related to the upgrade that
required variables in the rc which the RPM would not
automatically insert (per policy and for sanity in general!)
----
Thanks, and sorry for the trouble. If you have any questions
please feel free to ask.
Sitaram
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* [PATCH v3 1/3] http.c: Use curl_multi_fdset to select on curl fds instead of just sleeping
From: Mika Fischer @ 2011-11-04 14:19 UTC (permalink / raw)
To: git; +Cc: gitster, daniel, peff, Mika Fischer
In-Reply-To: <1320416367-28843-1-git-send-email-mika.fischer@zoopnet.de>
Instead of sleeping unconditionally for a 50ms, when no data can be read
from the http connection(s), use curl_multi_fdset to obtain the actual
file descriptors of the open connections and use them in the select call.
This way, the 50ms sleep is interrupted when new data arrives.
Signed-off-by: Mika Fischer <mika.fischer@zoopnet.de>
---
http.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/http.c b/http.c
index a4bc770..ae92318 100644
--- a/http.c
+++ b/http.c
@@ -664,14 +664,14 @@ void run_active_slot(struct active_request_slot *slot)
}
if (slot->in_use && !data_received) {
- max_fd = 0;
+ max_fd = -1;
FD_ZERO(&readfds);
FD_ZERO(&writefds);
FD_ZERO(&excfds);
+ curl_multi_fdset(curlm, &readfds, &writefds, &excfds, &max_fd);
select_timeout.tv_sec = 0;
select_timeout.tv_usec = 50000;
- select(max_fd, &readfds, &writefds,
- &excfds, &select_timeout);
+ select(max_fd+1, &readfds, &writefds, &excfds, &select_timeout);
}
}
#else
--
1.7.8.rc0.35.gd9f16.dirty
^ permalink raw reply related
* [PATCH v3 0/3] Improve use of select in http backend
From: Mika Fischer @ 2011-11-04 14:19 UTC (permalink / raw)
To: git; +Cc: gitster, daniel, peff, Mika Fischer
In-Reply-To: <1320265288-12647-1-git-send-email-mika.fischer@zoopnet.de>
Changes since v2:
- Properly signed off
- Incorporated Jeff's suggestion of getting rid of data_received
altogether
And yes, this is just a performance improvement, not a bugfix.
Mika Fischer (3):
http.c: Use curl_multi_fdset to select on curl fds instead of just
sleeping
http.c: Use timeout suggested by curl instead of fixed 50ms timeout
http.c: Rely on select instead of tracking whether data was received
http.c | 42 +++++++++++++++++++++++-------------------
http.h | 1 -
2 files changed, 23 insertions(+), 20 deletions(-)
--
1.7.8.rc0.35.gd9f16.dirty
^ permalink raw reply
* [PATCH v3 2/3] http.c: Use timeout suggested by curl instead of fixed 50ms timeout
From: Mika Fischer @ 2011-11-04 14:19 UTC (permalink / raw)
To: git; +Cc: gitster, daniel, peff, Mika Fischer
In-Reply-To: <1320416367-28843-1-git-send-email-mika.fischer@zoopnet.de>
Recent versions of curl can suggest a period of time the library user
should sleep and try again, when curl is blocked on reading or writing
(or connecting). Use this timeout instead of always sleeping for 50ms.
Signed-off-by: Mika Fischer <mika.fischer@zoopnet.de>
---
http.c | 22 ++++++++++++++++++++--
1 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/http.c b/http.c
index ae92318..e91a2ab 100644
--- a/http.c
+++ b/http.c
@@ -649,6 +649,9 @@ void run_active_slot(struct active_request_slot *slot)
fd_set excfds;
int max_fd;
struct timeval select_timeout;
+#if LIBCURL_VERSION_NUM >= 0x070f04
+ long curl_timeout;
+#endif
int finished = 0;
slot->finished = &finished;
@@ -664,13 +667,28 @@ void run_active_slot(struct active_request_slot *slot)
}
if (slot->in_use && !data_received) {
+#if LIBCURL_VERSION_NUM >= 0x070f04
+ curl_multi_timeout(curlm, &curl_timeout);
+ if (curl_timeout == 0) {
+ continue;
+ } else if (curl_timeout == -1) {
+ select_timeout.tv_sec = 0;
+ select_timeout.tv_usec = 50000;
+ } else {
+ select_timeout.tv_sec = curl_timeout / 1000;
+ select_timeout.tv_usec = (curl_timeout % 1000) * 1000;
+ }
+#else
+ select_timeout.tv_sec = 0;
+ select_timeout.tv_usec = 50000;
+#endif
+
max_fd = -1;
FD_ZERO(&readfds);
FD_ZERO(&writefds);
FD_ZERO(&excfds);
curl_multi_fdset(curlm, &readfds, &writefds, &excfds, &max_fd);
- select_timeout.tv_sec = 0;
- select_timeout.tv_usec = 50000;
+
select(max_fd+1, &readfds, &writefds, &excfds, &select_timeout);
}
}
--
1.7.8.rc0.35.gd9f16.dirty
^ permalink raw reply related
* [PATCH v3 3/3] http.c: Rely on select instead of tracking whether data was received
From: Mika Fischer @ 2011-11-04 14:19 UTC (permalink / raw)
To: git; +Cc: gitster, daniel, peff, Mika Fischer
In-Reply-To: <1320416367-28843-1-git-send-email-mika.fischer@zoopnet.de>
Since now select is used with the file descriptors of the http connections,
tracking whether data was received recently (and trying to read more in
that case) is no longer necessary. Instead, always call select and rely on
it to return as soon as new data can be read.
Signed-off-by: Mika Fischer <mika.fischer@zoopnet.de>
---
http.c | 16 +---------------
http.h | 1 -
2 files changed, 1 insertions(+), 16 deletions(-)
diff --git a/http.c b/http.c
index e91a2ab..3c6a00b 100644
--- a/http.c
+++ b/http.c
@@ -4,7 +4,6 @@
#include "run-command.h"
#include "url.h"
-int data_received;
int active_requests;
int http_is_verbose;
size_t http_post_buffer = 16 * LARGE_PACKET_MAX;
@@ -99,13 +98,11 @@ size_t fwrite_buffer(char *ptr, size_t eltsize, size_t nmemb, void *buffer_)
struct strbuf *buffer = buffer_;
strbuf_add(buffer, ptr, size);
- data_received++;
return size;
}
size_t fwrite_null(char *ptr, size_t eltsize, size_t nmemb, void *strbuf)
{
- data_received++;
return eltsize * nmemb;
}
@@ -642,8 +639,6 @@ void step_active_slots(void)
void run_active_slot(struct active_request_slot *slot)
{
#ifdef USE_CURL_MULTI
- long last_pos = 0;
- long current_pos;
fd_set readfds;
fd_set writefds;
fd_set excfds;
@@ -656,17 +651,9 @@ void run_active_slot(struct active_request_slot *slot)
slot->finished = &finished;
while (!finished) {
- data_received = 0;
step_active_slots();
- if (!data_received && slot->local != NULL) {
- current_pos = ftell(slot->local);
- if (current_pos > last_pos)
- data_received++;
- last_pos = current_pos;
- }
-
- if (slot->in_use && !data_received) {
+ if (slot->in_use) {
#if LIBCURL_VERSION_NUM >= 0x070f04
curl_multi_timeout(curlm, &curl_timeout);
if (curl_timeout == 0) {
@@ -1232,7 +1219,6 @@ static size_t fwrite_sha1_file(char *ptr, size_t eltsize, size_t nmemb,
git_SHA1_Update(&freq->c, expn,
sizeof(expn) - freq->stream.avail_out);
} while (freq->stream.avail_in && freq->zret == Z_OK);
- data_received++;
return size;
}
diff --git a/http.h b/http.h
index 3c332a9..71bdf58 100644
--- a/http.h
+++ b/http.h
@@ -89,7 +89,6 @@ extern void step_active_slots(void);
extern void http_init(struct remote *remote, const char *url);
extern void http_cleanup(void);
-extern int data_received;
extern int active_requests;
extern int http_is_verbose;
extern size_t http_post_buffer;
--
1.7.8.rc0.35.gd9f16.dirty
^ permalink raw reply related
* Re: New Feature wanted: Is it possible to let git clone continue last break point?
From: Shawn Pearce @ 2011-11-04 14:22 UTC (permalink / raw)
To: Johannes Sixt
Cc: Clemens Buchacher, Jeff King, Junio C Hamano, Jonathan Nieder,
netroby, Git Mail List, Tomas Carnecky
In-Reply-To: <4EB3B1E7.7080507@viscovery.net>
On Fri, Nov 4, 2011 at 02:35, Johannes Sixt <j.sixt@viscovery.net> wrote:
> Am 11/4/2011 9:56, schrieb Clemens Buchacher:
>> Cache ... not the pack but the information
>> to re-create it...
>
> It has been discussed. It doesn't work. Because with threaded pack
> generation, the resulting pack is not deterministic.
The information to create a pack for a repository with 2M objects
(e.g. Linux kernel tree) is *at least* 152M of data. This is just a
first order approximation of what it takes to write out the 2M SHA-1s,
along with say a 4 byte length so you can find given an offset
provided by the client roughly where to resumse in the object stream.
This is like 25% of the pack size itself. Ouch.
This data is still insufficient to resume from. A correct solution
would allow you to resume in the middle of an object, which means we
also need to store some sort of indicator of which representation was
chosen from an existing pack file for object reuse. Which adds more
data to the stream. And then there is the not so simple problem of how
to resume in the middle of an object that was being recompressed on
the fly, such as a large loose object.
By the time you get done with all of that, your "ticket" might as well
be the name of a pack file. And your "resume information" is just a
pack file itself. Which would be very expensive to recreate.
^ permalink raw reply
* Re: [git patches] libata updates, GPG signed (but see admin notes)
From: Ted Ts'o @ 2011-11-04 14:59 UTC (permalink / raw)
To: Linus Torvalds
Cc: Junio C Hamano, Shawn Pearce, git, James Bottomley, Jeff Garzik,
Andrew Morton, linux-ide, LKML
In-Reply-To: <CA+55aFyRawm9CoJMiEXDFCX4YTidPOiV4oqSS2d7nNv7Ecw8BQ@mail.gmail.com>
On Thu, Nov 03, 2011 at 12:09:55PM -0700, Linus Torvalds wrote:
> I personally dislike it, and don't really think it's a wonderful thing
> at all. I really does have real downsides:
>
> - internal signatures really *are* a disaster for maintenance. You
> can never fix them if they need fixing (and "need fixing" may well be
> "you want to re-sign things after a repository format change")
Note that a repository format change will break a bunch of other
things as well, including references in commit descriptions ("This
fixes a regression introduced in commit 42DEADBEEF") So if SHA-1 is in
danger of failing in way that would threaten git's use of it (highly
unlikely), we'd probably be well advised to find a way to add a new
crypto checksum (i.e., SHA-256) in parallel, but keep the original
SHA-1 checksum for UI purposes.
> - they are ugly as heck, and you really don't want to see them in
> 99.999% of all cases.
So we can make them be hidden from "git log" and "gik" by default.
That bit is a bit gross, I agree, but 3rd party verification really is
a good thing, which I'm hoping can be added in a relatively clean
fashion.
- Ted
^ permalink raw reply
* Re: [git patches] libata updates, GPG signed (but see admin notes)
From: Linus Torvalds @ 2011-11-04 15:14 UTC (permalink / raw)
To: Ted Ts'o, Linus Torvalds, Junio C Hamano, Shawn Pearce, git,
James
In-Reply-To: <20111104145908.GA3903@thunk.org>
On Fri, Nov 4, 2011 at 7:59 AM, Ted Ts'o <tytso@mit.edu> wrote:
>
> Note that a repository format change will break a bunch of other
> things as well, including references in commit descriptions ("This
> fixes a regression introduced in commit 42DEADBEEF")
No they won't. Not if you do it right. It's easy enough to
automatically replace the SHA1's in the description, the same way we
replace everything else.
Really. It's *trivial*.
Maybe some current tools don't do it, but if I were to convert the
kernel tree, I'd absolutely *require* the conversion to be done right.
And "right" means "don't just get the parent SHA1's right, but the
ones hiding in the description too".
Any conversion tool has to keep track of the translation from "old
SHA1 to new SHA1" *anyway* because of all the other issues (ie exactly
things like parent pointers etc), so conversion tools by definition
have the information to do things like this right.
But "internal cryptographic signatures" are fundamentally different. A
conversion tool *cannot* convert them, since it won't have access to
the private keys in question, and thus cannot fix up the signature.
Sure, if I do the conversion, I could make *my* signatures match. And
that is true for every signer out there - individually. But only
individually, never collectively. Sure, we could all meet in one place
and synchronously re-sign things on our private machines with some
"distributed conversion tool", but realistically that really really
doesn't work.
It's a fundamental problem. And it really isn't a theoretical one -
it's one we know will happen *some* day.
I haven't worried about SHA1, exactly because I know it's not a real
problem - we can always convert. But internal signatures very
fundamentally change that.
And it really is about *internal* signatures. The kinds of signed tags
we have now are not a problem. Those can trivially be converted in a
distributed manner, exactly because they are "detatched" from what
they sign. We carry them along with the git repo, but they don't mess
up history, and they can be re-created individually without changing
anything else.
And yes, this was actually a design issue for me, which is why I feel
so strongly about it. I actually *thought* about issues like this
five+ years ago: I wanted to have cryptographic security, but I very
much on purpose wanted it to be "outside" the repo.
(Ok, so the git tag objects can sign other git tag objects
recursively, and in that case you have an ordering issue where a
conversion would first have to get somebody to re-sign their "inner"
tag before the "outer" signature can be re-created, but even if that
were to happen - and I don't think anybody does it - it's a trivial
problem with no real complexity issues).
>> - they are ugly as heck, and you really don't want to see them in
>> 99.999% of all cases.
>
> So we can make them be hidden from "git log" and "gik" by default.
> That bit is a bit gross, I agree, but 3rd party verification really is
> a good thing, which I'm hoping can be added in a relatively clean
> fashion.
I agree that we can hide them - that's after all what the pgpsig thing
does in the "internal commit signature" that git has in pu/next. That
one hides ie even more specifically, by putting it in the headers of
the commit, but that's just a random implementation detail.
But I really think that "internal signatures" that actually affect the
SHA1 of the object and its history have fundamental design problems.
They may not be "insurmountably bad", but they are definitely real.
Linus
^ permalink raw reply
* Re: [msysGit] Re: [PATCHv2] Compile fix for MSVC
From: Sebastian Schuberth @ 2011-11-04 15:39 UTC (permalink / raw)
To: Pat Thoyts
Cc: Vincent van Ravesteijn, Johannes Schindelin, ramsay, msysgit,
Karsten Blees, git, Junio C Hamano, Erik Faye-Lund
In-Reply-To: <CABNJ2G+Km4wob4_uNYQLkQUL61-ohZg5cL2yu7j1cngoL9W7Cw@mail.gmail.com>
On 02.11.2011 01:48, Pat Thoyts wrote:
>> As long as this means to just run a not so complicated perl script, this
>> Could even be done in a commit hook.
>>
>> Just another question. How does the (msys)git community feel about adding
>> CMake support ? I can probably do that quite easily.
>
> -1. We have a make. We don't need two of them.
I don't see CMake as an alternative to make as CMake would be used to
generate the Makefile, which is currently hard-coded and committed.
However, I don't see any benefit in using CMake as long as the Linux
guys don't use it to create their Makefile, too.
--
Sebastian Schuberth
^ permalink raw reply
* [PATCH 0/4] fsck improvements
From: Nguyễn Thái Ngọc Duy @ 2011-11-04 15:47 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
So I looked at fsck to see if it's easy to support multithread and
found out I could reduce fsck time by half. So here it is. I think I
haven't made any mistakes but fsck is not my domain.
The last patch is reposted because it conflicts with the series. It
also prints progress when checking connnectivity.
Nguyễn Thái Ngọc Duy (4):
fsck: return error code when verify_pack() goes wrong
Stop verify_packfile() as soon as an error occurs
fsck: avoid reading every object twice
fsck: print progress
Documentation/git-fsck.txt | 12 ++++++-
builtin/fsck.c | 78 +++++++++++++++++++++++++++++++++++---------
pack-check.c | 27 +++++++++++++--
pack.h | 6 +++-
4 files changed, 101 insertions(+), 22 deletions(-)
--
1.7.4.74.g639db
^ permalink raw reply
* [PATCH 1/4] fsck: return error code when verify_pack() goes wrong
From: Nguyễn Thái Ngọc Duy @ 2011-11-04 15:47 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1320421670-518-1-git-send-email-pclouds@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
builtin/fsck.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/builtin/fsck.c b/builtin/fsck.c
index df1a88b..4ead98d 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -29,6 +29,7 @@ static int write_lost_and_found;
static int verbose;
#define ERROR_OBJECT 01
#define ERROR_REACHABLE 02
+#define ERROR_PACK 04
#ifdef NO_D_INO_IN_DIRENT
#define SORT_DIRENT 0
@@ -626,7 +627,8 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)
prepare_packed_git();
for (p = packed_git; p; p = p->next)
/* verify gives error messages itself */
- verify_pack(p);
+ if (verify_pack(p))
+ errors_found |= ERROR_PACK;
for (p = packed_git; p; p = p->next) {
uint32_t j, num;
--
1.7.4.74.g639db
^ permalink raw reply related
* [PATCH 2/4] Stop verify_packfile() as soon as an error occurs
From: Nguyễn Thái Ngọc Duy @ 2011-11-04 15:47 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1320421670-518-1-git-send-email-pclouds@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
pack-check.c | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/pack-check.c b/pack-check.c
index 0c19b6e..e33ea79 100644
--- a/pack-check.c
+++ b/pack-check.c
@@ -79,6 +79,8 @@ static int verify_packfile(struct packed_git *p,
err = error("%s SHA1 does not match its index",
p->pack_name);
unuse_pack(w_curs);
+ if (err)
+ return err;
/* Make sure everything reachable from idx is valid. Since we
* have verified that nr_objects matches between idx and pack,
@@ -106,11 +108,13 @@ static int verify_packfile(struct packed_git *p,
off_t offset = entries[i].offset;
off_t len = entries[i+1].offset - offset;
unsigned int nr = entries[i].nr;
- if (check_pack_crc(p, w_curs, offset, len, nr))
+ if (check_pack_crc(p, w_curs, offset, len, nr)) {
err = error("index CRC mismatch for object %s "
"from %s at offset %"PRIuMAX"",
sha1_to_hex(entries[i].sha1),
p->pack_name, (uintmax_t)offset);
+ break;
+ }
}
data = unpack_entry(p, entries[i].offset, &type, &size);
if (!data) {
--
1.7.4.74.g639db
^ permalink raw reply related
* [PATCH 3/4] fsck: avoid reading every object twice
From: Nguyễn Thái Ngọc Duy @ 2011-11-04 15:47 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1320421670-518-1-git-send-email-pclouds@gmail.com>
During verify_pack() all objects are read for SHA-1 check. Then
fsck_sha1() is called on every object, which read the object again
(fsck_sha1 -> parse_object -> read_sha1_file).
Avoid reading an object twice, do fsck_sha1 while we have an object
uncompressed data in verify_pack.
On git.git, with this patch I got:
$ /usr/bin/time ./git fsck >/dev/null
98.97user 0.90system 1:40.01elapsed 99%CPU (0avgtext+0avgdata 616624maxresident)k
0inputs+0outputs (0major+194186minor)pagefaults 0swaps
Without it:
$ /usr/bin/time ./git fsck >/dev/null
231.23user 2.35system 3:53.82elapsed 99%CPU (0avgtext+0avgdata 636688maxresident)k
0inputs+0outputs (0major+461629minor)pagefaults 0swaps
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
builtin/fsck.c | 42 +++++++++++++++++++++++++-----------------
pack-check.c | 13 ++++++++++---
pack.h | 5 ++++-
3 files changed, 39 insertions(+), 21 deletions(-)
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 4ead98d..0603f64 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -282,14 +282,8 @@ static void check_connectivity(void)
}
}
-static int fsck_sha1(const unsigned char *sha1)
+static int fsck_obj(struct object *obj)
{
- struct object *obj = parse_object(sha1);
- if (!obj) {
- errors_found |= ERROR_OBJECT;
- return error("%s: object corrupt or missing",
- sha1_to_hex(sha1));
- }
if (obj->flags & SEEN)
return 0;
obj->flags |= SEEN;
@@ -332,6 +326,29 @@ static int fsck_sha1(const unsigned char *sha1)
return 0;
}
+static int fsck_sha1(const unsigned char *sha1)
+{
+ struct object *obj = parse_object(sha1);
+ if (!obj) {
+ errors_found |= ERROR_OBJECT;
+ return error("%s: object corrupt or missing",
+ sha1_to_hex(sha1));
+ }
+ return fsck_obj(obj);
+}
+
+static int fsck_obj_buffer(const unsigned char *sha1, enum object_type type,
+ unsigned long size, void *buffer, int *eaten)
+{
+ struct object *obj;
+ obj = parse_object_buffer(sha1, type, size, buffer, eaten);
+ if (!obj) {
+ errors_found |= ERROR_OBJECT;
+ return error("%s: object corrupt or missing", sha1_to_hex(sha1));
+ }
+ return fsck_obj(obj);
+}
+
/*
* This is the sorting chunk size: make it reasonably
* big so that we can sort well..
@@ -627,17 +644,8 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)
prepare_packed_git();
for (p = packed_git; p; p = p->next)
/* verify gives error messages itself */
- if (verify_pack(p))
+ if (verify_pack(p, fsck_obj_buffer))
errors_found |= ERROR_PACK;
-
- for (p = packed_git; p; p = p->next) {
- uint32_t j, num;
- if (open_pack_index(p))
- continue;
- num = p->num_objects;
- for (j = 0; j < num; j++)
- fsck_sha1(nth_packed_object_sha1(p, j));
- }
}
heads = 0;
diff --git a/pack-check.c b/pack-check.c
index e33ea79..372d6b2 100644
--- a/pack-check.c
+++ b/pack-check.c
@@ -42,7 +42,8 @@ int check_pack_crc(struct packed_git *p, struct pack_window **w_curs,
}
static int verify_packfile(struct packed_git *p,
- struct pack_window **w_curs)
+ struct pack_window **w_curs,
+ verify_fn fn)
{
off_t index_size = p->index_size;
const unsigned char *index_base = p->index_data;
@@ -129,6 +130,12 @@ static int verify_packfile(struct packed_git *p,
free(data);
break;
}
+ if (fn) {
+ int eaten = 0;
+ fn(entries[i].sha1, type, size, data, &eaten);
+ if (eaten)
+ data = NULL;
+ }
free(data);
}
free(entries);
@@ -159,7 +166,7 @@ int verify_pack_index(struct packed_git *p)
return err;
}
-int verify_pack(struct packed_git *p)
+int verify_pack(struct packed_git *p, verify_fn fn)
{
int err = 0;
struct pack_window *w_curs = NULL;
@@ -168,7 +175,7 @@ int verify_pack(struct packed_git *p)
if (!p->index_data)
return -1;
- err |= verify_packfile(p, &w_curs);
+ err |= verify_packfile(p, &w_curs, fn);
unuse_pack(&w_curs);
return err;
diff --git a/pack.h b/pack.h
index 722a54e..70f3c29 100644
--- a/pack.h
+++ b/pack.h
@@ -70,10 +70,13 @@ struct pack_idx_entry {
off_t offset;
};
+
+typedef int (*verify_fn)(const unsigned char*, enum object_type, unsigned long, void*, int*);
+
extern const char *write_idx_file(const char *index_name, struct pack_idx_entry **objects, int nr_objects, const struct pack_idx_option *, unsigned char *sha1);
extern int check_pack_crc(struct packed_git *p, struct pack_window **w_curs, off_t offset, off_t len, unsigned int nr);
extern int verify_pack_index(struct packed_git *);
-extern int verify_pack(struct packed_git *);
+extern int verify_pack(struct packed_git *, verify_fn fn);
extern void fixup_pack_header_footer(int, unsigned char *, const char *, uint32_t, unsigned char *, off_t);
extern char *index_pack_lockfile(int fd);
extern int encode_in_pack_object_header(enum object_type, uintmax_t, unsigned char *);
--
1.7.4.74.g639db
^ permalink raw reply related
* [PATCH 4/4] fsck: print progress
From: Nguyễn Thái Ngọc Duy @ 2011-11-04 15:47 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1320421670-518-1-git-send-email-pclouds@gmail.com>
fsck is usually a long process and it would be nice if it prints
progress from time to time.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
Documentation/git-fsck.txt | 12 +++++++++++-
builtin/fsck.c | 40 ++++++++++++++++++++++++++++++++++++++--
pack-check.c | 14 +++++++++++---
pack.h | 3 ++-
4 files changed, 62 insertions(+), 7 deletions(-)
diff --git a/Documentation/git-fsck.txt b/Documentation/git-fsck.txt
index a2a508d..5245101 100644
--- a/Documentation/git-fsck.txt
+++ b/Documentation/git-fsck.txt
@@ -10,7 +10,8 @@ SYNOPSIS
--------
[verse]
'git fsck' [--tags] [--root] [--unreachable] [--cache] [--no-reflogs]
- [--[no-]full] [--strict] [--verbose] [--lost-found] [<object>*]
+ [--[no-]full] [--strict] [--verbose] [--lost-found]
+ [--[no-]progress] [<object>*]
DESCRIPTION
-----------
@@ -72,6 +73,15 @@ index file, all SHA1 references in .git/refs/*, and all reflogs (unless
a blob, the contents are written into the file, rather than
its object name.
+--progress::
+--no-progress::
+ When fsck is run in a terminal, it will show the progress.
+ These options can force progress to be shown or not
+ regardless terminal check.
++
+Progress is not shown when --verbose is used. --progress is ignored
+in this case.
+
It tests SHA1 and general object sanity, and it does full tracking of
the resulting reachability and everything else. It prints out any
corruption it finds (missing or bad objects), and if you use the
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 0603f64..c4b1ca6 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -11,6 +11,7 @@
#include "fsck.h"
#include "parse-options.h"
#include "dir.h"
+#include "progress.h"
#define REACHABLE 0x0001
#define SEEN 0x0002
@@ -27,6 +28,7 @@ static const char *head_points_at;
static int errors_found;
static int write_lost_and_found;
static int verbose;
+static int show_progress = -1;
#define ERROR_OBJECT 01
#define ERROR_REACHABLE 02
#define ERROR_PACK 04
@@ -138,7 +140,11 @@ static int traverse_one_object(struct object *obj)
static int traverse_reachable(void)
{
+ struct progress *progress = NULL;
+ unsigned int nr = 0;
int result = 0;
+ if (show_progress)
+ progress = start_progress_delay("Checking connectivity", 0, 0, 2);
while (pending.nr) {
struct object_array_entry *entry;
struct object *obj;
@@ -146,7 +152,9 @@ static int traverse_reachable(void)
entry = pending.objects + --pending.nr;
obj = entry->item;
result |= traverse_one_object(obj);
+ display_progress(progress, ++nr);
}
+ stop_progress(&progress);
return !!result;
}
@@ -530,15 +538,20 @@ static void get_default_heads(void)
static void fsck_object_dir(const char *path)
{
int i;
+ struct progress *progress = NULL;
if (verbose)
fprintf(stderr, "Checking object directory\n");
+ if (show_progress)
+ progress = start_progress("Checking object directories", 256);
for (i = 0; i < 256; i++) {
static char dir[4096];
sprintf(dir, "%s/%02x", path, i);
fsck_dir(i, dir);
+ display_progress(progress, i+1);
}
+ stop_progress(&progress);
fsck_sha1_list();
}
@@ -609,6 +622,7 @@ static struct option fsck_opts[] = {
OPT_BOOLEAN(0, "strict", &check_strict, "enable more strict checking"),
OPT_BOOLEAN(0, "lost-found", &write_lost_and_found,
"write dangling objects in .git/lost-found"),
+ OPT_BOOL (0, "progress", &show_progress, "show progress"),
OPT_END(),
};
@@ -621,6 +635,12 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)
read_replace_refs = 0;
argc = parse_options(argc, argv, prefix, fsck_opts, fsck_usage, 0);
+
+ if (show_progress == -1)
+ show_progress = isatty(2);
+ if (verbose)
+ show_progress = 0;
+
if (write_lost_and_found) {
check_full = 1;
include_reflogs = 0;
@@ -640,12 +660,28 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)
if (check_full) {
struct packed_git *p;
+ uint32_t total = 0, count = 0;
+ struct progress *progress = NULL;
prepare_packed_git();
- for (p = packed_git; p; p = p->next)
+
+ if (show_progress) {
+ for (p = packed_git; p; p = p->next) {
+ if (open_pack_index(p))
+ continue;
+ total += p->num_objects;
+ }
+
+ progress = start_progress("Checking objects", total);
+ }
+ for (p = packed_git; p; p = p->next) {
/* verify gives error messages itself */
- if (verify_pack(p, fsck_obj_buffer))
+ if (verify_pack(p, fsck_obj_buffer,
+ progress, count))
errors_found |= ERROR_PACK;
+ count += p->num_objects;
+ }
+ stop_progress(&progress);
}
heads = 0;
diff --git a/pack-check.c b/pack-check.c
index 372d6b2..a3262af 100644
--- a/pack-check.c
+++ b/pack-check.c
@@ -1,6 +1,7 @@
#include "cache.h"
#include "pack.h"
#include "pack-revindex.h"
+#include "progress.h"
struct idx_entry {
off_t offset;
@@ -43,7 +44,9 @@ int check_pack_crc(struct packed_git *p, struct pack_window **w_curs,
static int verify_packfile(struct packed_git *p,
struct pack_window **w_curs,
- verify_fn fn)
+ verify_fn fn,
+ struct progress *progress, uint32_t base_count)
+
{
off_t index_size = p->index_size;
const unsigned char *index_base = p->index_data;
@@ -136,8 +139,12 @@ static int verify_packfile(struct packed_git *p,
if (eaten)
data = NULL;
}
+ if (((base_count + i) & 1023) == 0)
+ display_progress(progress, base_count + i);
free(data);
+
}
+ display_progress(progress, base_count + i);
free(entries);
return err;
@@ -166,7 +173,8 @@ int verify_pack_index(struct packed_git *p)
return err;
}
-int verify_pack(struct packed_git *p, verify_fn fn)
+int verify_pack(struct packed_git *p, verify_fn fn,
+ struct progress *progress, uint32_t base_count)
{
int err = 0;
struct pack_window *w_curs = NULL;
@@ -175,7 +183,7 @@ int verify_pack(struct packed_git *p, verify_fn fn)
if (!p->index_data)
return -1;
- err |= verify_packfile(p, &w_curs, fn);
+ err |= verify_packfile(p, &w_curs, fn, progress, base_count);
unuse_pack(&w_curs);
return err;
diff --git a/pack.h b/pack.h
index 70f3c29..324a1d7 100644
--- a/pack.h
+++ b/pack.h
@@ -71,12 +71,13 @@ struct pack_idx_entry {
};
+struct progress;
typedef int (*verify_fn)(const unsigned char*, enum object_type, unsigned long, void*, int*);
extern const char *write_idx_file(const char *index_name, struct pack_idx_entry **objects, int nr_objects, const struct pack_idx_option *, unsigned char *sha1);
extern int check_pack_crc(struct packed_git *p, struct pack_window **w_curs, off_t offset, off_t len, unsigned int nr);
extern int verify_pack_index(struct packed_git *);
-extern int verify_pack(struct packed_git *, verify_fn fn);
+extern int verify_pack(struct packed_git *, verify_fn fn, struct progress *, uint32_t);
extern void fixup_pack_header_footer(int, unsigned char *, const char *, uint32_t, unsigned char *, off_t);
extern char *index_pack_lockfile(int fd);
extern int encode_in_pack_object_header(enum object_type, uintmax_t, unsigned char *);
--
1.7.4.74.g639db
^ permalink raw reply related
* Re: New Feature wanted: Is it possible to let git clone continue last break point?
From: Jakub Narebski @ 2011-11-04 15:55 UTC (permalink / raw)
To: Shawn Pearce
Cc: Johannes Sixt, Clemens Buchacher, Jeff King, Junio C Hamano,
Jonathan Nieder, netroby, Git Mail List, Tomas Carnecky
In-Reply-To: <CAJo=hJtsiEEHA33CQn1MCvb7vFv7uEF+U292YgBa7EWv7P8Jng@mail.gmail.com>
Shawn Pearce <spearce@spearce.org> writes:
> On Fri, Nov 4, 2011 at 02:35, Johannes Sixt <j.sixt@viscovery.net> wrote:
> > Am 11/4/2011 9:56, schrieb Clemens Buchacher:
> > > Cache ... not the pack but the information
> > > to re-create it...
> >
> > It has been discussed. It doesn't work. Because with threaded pack
> > generation, the resulting pack is not deterministic.
>
> The information to create a pack for a repository with 2M objects
> (e.g. Linux kernel tree) is *at least* 152M of data. This is just a
> first order approximation of what it takes to write out the 2M SHA-1s,
> along with say a 4 byte length so you can find given an offset
> provided by the client roughly where to resumse in the object stream.
> This is like 25% of the pack size itself. Ouch.
Well, perhaps caching a few most popular packs in some kind of cache
(packfile is saved to disk as it is streamed if we detect that it will
be large), indexing by WANT / HAVE?
> This data is still insufficient to resume from. A correct solution
> would allow you to resume in the middle of an object, which means we
> also need to store some sort of indicator of which representation was
> chosen from an existing pack file for object reuse. Which adds more
> data to the stream. And then there is the not so simple problem of how
> to resume in the middle of an object that was being recompressed on
> the fly, such as a large loose object.
Well, so you wouldn't be able to just concatenate packs^W received
data. Still it should be possible to "repair" halfway downloaded
partial pack...
Just my 2 eurocents^W groszy.
--
Jakub Narębski
^ permalink raw reply
* Re: Folder level Acces in git
From: Eugene Sajine @ 2011-11-04 15:59 UTC (permalink / raw)
To: Joshua Jensen; +Cc: redhat1981, git
In-Reply-To: <4EB36855.8000802@workspacewhiz.com>
On Fri, Nov 4, 2011 at 12:21 AM, Joshua Jensen
<jjensen@workspacewhiz.com> wrote:
> ----- Original Message -----
> From: Eugene Sajine
> Date: 11/3/2011 1:28 PM
>>
>> Are you sure that the way your have organized the repository is
>> actually correct? If you need to manage the access on folder level why
>> don't you simply split up the project into several
>> repositories/projects which each team is going to work with
>> independently?
>>
>> This seems to me to be much simpler and cleaner solution then any
>> other alternative.
>>
> Submodules are _not_ simple at all. Our organization of nearly 100
> developers using Git pretty much let out a collective cheer when we finally
> removed the submodules. Submodules are an absolute pain to develop within;
> there are a number of Git mailing list exchanges about that, but I'd be
> happy to go into great detail if anybody cares. Even submodules that are
> read-only are a pain as it takes two steps (git pull + git submodule update)
> to actually get them up to date.
>
> Ick.
>
> In short, if it is an absolute requirement to manage access to a repository
> on a folder level, get Subversion or Perforce. DVCS is not for you...
>
> Josh
>
That is exactly what i wanted to say. I suggest OP not to go into
submodules, but just have separate repositories. I think if they need
this kind of granularity in permissions it means that their repository
is too big and incorrectly organized. I think they are trying to
migrate to better VCS (as git is superior by definition;) ), but they
still think in central VCS terms and that's what causing this folder
level management requirement to appear. We at $work have hundreds of
repositories and never had any need of submodules or folder level
permissions. (One repo = one project = one artifact) + Ivy as
dependency manager works just fine and if we will need to fine tune
the permissions it will be always pretty easy to do.
Thanks,
Eugene
^ permalink raw reply
* Re: New Feature wanted: Is it possible to let git clone continue last break point?
From: Nguyen Thai Ngoc Duy @ 2011-11-04 16:05 UTC (permalink / raw)
To: Shawn Pearce
Cc: Johannes Sixt, Clemens Buchacher, Jeff King, Junio C Hamano,
Jonathan Nieder, netroby, Git Mail List, Tomas Carnecky
In-Reply-To: <CAJo=hJtsiEEHA33CQn1MCvb7vFv7uEF+U292YgBa7EWv7P8Jng@mail.gmail.com>
2011/11/4 Shawn Pearce <spearce@spearce.org>:
> By the time you get done with all of that, your "ticket" might as well
> be the name of a pack file. And your "resume information" is just a
> pack file itself. Which would be very expensive to recreate.
I'll deal with initial clone case only here. Can we make git protocol
send multiple packs, then send on-disk packs one by one together with
pack SHA1? This way we do not need to recreate anything. If new packs
are created during cloning, git client should be able to construct
"have" list from good packs and fetch updates from server again.
--
Duy
^ permalink raw reply
* Re: [PATCHv2] Add options to specify snapshot file name, prefix
From: Jakub Narebski @ 2011-11-04 16:10 UTC (permalink / raw)
To: Thomas Guyot-Sionnest; +Cc: git, Jakub Narebski
In-Reply-To: <1320367999-24435-1-git-send-email-dermoth@aei.ca>
Thomas Guyot-Sionnest <dermoth@aei.ca> writes:
> commit b629275 implemented "smart" snapshot names and prefixes. I have
> scripts that used to rely on the old behaviour which allowed in some
> cases to have fixed prefix, and would require modifications to work with
> newer Gitweb.
If scripts use 'wget' or 'curl' you can always change the name of
saved file:
wget -O <file> ...
curl -o <file> ...
If downloaded snapshot is compressed tarfile, you can use
--strip-components=1 to strip prefix.
> This patch adds two parameters for overriding the snapshot name and
> prefix, sn and sp respectively. For example, to get a snapshot
> named "myproject.[suffix]" with no prefix one can add this query string:
> ?sn=myproject;sp=
Would you need support for expandable parameters in both (a la
'action' feature)?
[...]
> @@ -6684,11 +6686,19 @@ sub git_snapshot {
> }
>
> my ($name, $prefix) = snapshot_name($project, $hash);
> + if (defined($input_params{'snapshot_name'})) {
> + $name = $input_params{'snapshot_name'};
> + }
> + if (defined($input_params{'snapshot_prefix'})) {
> + $prefix = $input_params{'snapshot_prefix'};
> + } else {
> + $prefix .= '/';
> + }
> my $filename = "$name$known_snapshot_formats{$format}{'suffix'}";
> my $cmd = quote_command(
> git_cmd(), 'archive',
> "--format=$known_snapshot_formats{$format}{'format'}",
> - "--prefix=$prefix/", $hash);
> + "--prefix=$prefix", $hash);
> if (exists $known_snapshot_formats{$format}{'compressor'}) {
> $cmd .= ' | ' . quote_command(@{$known_snapshot_formats{$format}{'compressor'}});
> }
I wonder if you really want to allow prefix which do not end in '/'
(which would be suprising, isn't it), or just allow empty prefix too.
For example
@@ -6684,11 +6686,19 @@ sub git_snapshot {
}
my ($name, $prefix) = snapshot_name($project, $hash);
+ if (defined($input_params{'snapshot_name'})) {
+ $name = $input_params{'snapshot_name'};
+ }
+ if (defined($input_params{'snapshot_prefix'})) {
+ $prefix = $input_params{'snapshot_prefix'};
+ }
my $filename = "$name$known_snapshot_formats{$format}{'suffix'}";
my $cmd = quote_command(
git_cmd(), 'archive',
"--format=$known_snapshot_formats{$format}{'format'}",
- "--prefix=$prefix/", $hash);
+ ($prefix eq "" ? () : "--prefix=$prefix"), $hash);
if (exists $known_snapshot_formats{$format}{'compressor'}) {
$cmd .= ' | ' . quote_command(@{$known_snapshot_formats{$format}{'compressor'}});
}
--
Jakub Narębski
^ permalink raw reply
* Re: [PATCH] http-push: don't always prompt for password
From: Junio C Hamano @ 2011-11-04 16:48 UTC (permalink / raw)
To: Stefan Naewe; +Cc: git, peff
In-Reply-To: <1320390188-24334-1-git-send-email-stefan.naewe@gmail.com>
Stefan Naewe <stefan.naewe@gmail.com> writes:
> http-push prompts for a password when the URL is set as
> 'https://user@host/repo' even though there is one set
> in ~/.netrc. Pressing ENTER at the password prompt succeeds
> then, but is a annoying and makes it almost useless
> in a shell script, e.g.
>
> Signed-off-by: Stefan Naewe <stefan.naewe@gmail.com>
> ---
Thanks.
With this the only callsite of init_curl_http_auth() becomes the one after
we get the 401 response, and this caller makes sure that user_name is not
NULL.
Do we still want "if (user_name)" inside init_curl_http_auth()?
I tried to rewrite the proposed commit log message to describe the real
issue, and here is what I came up with:
Author: Stefan Naewe <stefan.naewe@gmail.com>
Date: Fri Nov 4 08:03:08 2011 +0100
http: don't always prompt for password
When a username is already specified at the beginning of any HTTP
transaction (e.g. "git push https://user@hosting.example.com/project.git"
or "git ls-remote https://user@hosting.example.com/project.git"), the code
interactively asks for a password before calling into the libcurl library.
It is very likely that the reason why user included the username in the
URL is because the user knows that it would require authentication to
access the resource. Asking for the password upfront would save one
roundtrip to get a 401 response, getting the password and then retrying
the request. This is a reasonable optimization.
HOWEVER.
This is done even when $HOME/.netrc might have a corresponding entry to
access the site, or the site does not require authentication to access the
resource after all. But neither condition can be determined until we call
into libcurl library (we do not read and parse $HOME/.netrc ourselves). In
these cases, the user is forced to respond to the password prompt, only to
give a password that is not used in the HTTP transaction. If the password
is in $HOME/.netrc, an empty input would later let the libcurl layer to
pick up the password from there, and if the resource does not require
authentication, any input would be taken and then discarded without
getting used. It is wasteful to ask this unused information to the end
user.
Reduce the confusion by not trying to optimize for this case and always
incur roundtrip penalty. An alternative might be to document this and keep
this round-trip optimization as-is.
Signed-off-by: Stefan Naewe <stefan.naewe@gmail.com>
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
What is somewhat troubling is that after analyzing the root cause of the
issue, I am wondering if a more correct fix is to remove the user@ part
from the URL (in other words, document that a URL with an embedded
username will ask for password upfront, and tell the users that if they
have netrc entries or if they are accessing a resource that does not
require authentication, they should omit the username from the URL).
^ permalink raw reply
* Re: [PATCH v3 2/3] http.c: Use timeout suggested by curl instead of fixed 50ms timeout
From: Junio C Hamano @ 2011-11-04 17:13 UTC (permalink / raw)
To: Mika Fischer; +Cc: git, daniel, peff
In-Reply-To: <1320416367-28843-3-git-send-email-mika.fischer@zoopnet.de>
Mika Fischer <mika.fischer@zoopnet.de> writes:
> Recent versions of curl can suggest a period of time the library user
> should sleep and try again, when curl is blocked on reading or writing
> (or connecting). Use this timeout instead of always sleeping for 50ms.
>
> Signed-off-by: Mika Fischer <mika.fischer@zoopnet.de>
Thanks.
I'm inclined to squash in the following to narrow the scope of
curl_timeout, though.
diff --git a/http.c b/http.c
index 5cb0fb6..924be52 100644
--- a/http.c
+++ b/http.c
@@ -636,9 +636,6 @@ void run_active_slot(struct active_request_slot *slot)
fd_set excfds;
int max_fd;
struct timeval select_timeout;
-#if LIBCURL_VERSION_NUM >= 0x070f04
- long curl_timeout;
-#endif
int finished = 0;
slot->finished = &finished;
@@ -655,6 +652,7 @@ void run_active_slot(struct active_request_slot *slot)
if (slot->in_use && !data_received) {
#if LIBCURL_VERSION_NUM >= 0x070f04
+ long curl_timeout;
curl_multi_timeout(curlm, &curl_timeout);
if (curl_timeout == 0) {
continue;
^ permalink raw reply related
* Re: [PATCH] http-push: don't always prompt for password
From: Jeff King @ 2011-11-04 17:43 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Stefan Naewe, git
In-Reply-To: <7vlirvdeb2.fsf@alter.siamese.dyndns.org>
On Fri, Nov 04, 2011 at 09:48:17AM -0700, Junio C Hamano wrote:
> Stefan Naewe <stefan.naewe@gmail.com> writes:
>
> > http-push prompts for a password when the URL is set as
> > 'https://user@host/repo' even though there is one set
> > in ~/.netrc. Pressing ENTER at the password prompt succeeds
> > then, but is a annoying and makes it almost useless
> > in a shell script, e.g.
> >
> > Signed-off-by: Stefan Naewe <stefan.naewe@gmail.com>
> > ---
>
> Thanks.
>
> With this the only callsite of init_curl_http_auth() becomes the one after
> we get the 401 response, and this caller makes sure that user_name is not
> NULL.
>
> Do we still want "if (user_name)" inside init_curl_http_auth()?
Since we now only call init_curl_http_auth when we know we need auth, I
think it would make more sense to just move the user_name asking there,
too, like:
static void init_curl_http_auth(CURL *result)
{
struct strbuf up = STRBUF_INIT;
if (!user_name)
user_name = xstrdup(git_getpass_with_description("Username", description);
if (!user_pass)
user_pass = xstrdup(git_getpass_with_description("Password", description);
strbuf_addf(&up, "%s:%s", user_name, user_pass);
curl_easy_setopt(result, CURLOPT_USERPWD, strbuf_detach(&up, NULL));
}
And then it's easy to swap out the asking for credential_fill() when it
becomes available. But I admit I don't care that much now, as I'll just
end up doing that refactoring later with my credential patches anyway.
> I tried to rewrite the proposed commit log message to describe the real
> issue, and here is what I came up with:
Your description looks accurate to me.
> What is somewhat troubling is that after analyzing the root cause of the
> issue, I am wondering if a more correct fix is to remove the user@ part
> from the URL (in other words, document that a URL with an embedded
> username will ask for password upfront, and tell the users that if they
> have netrc entries or if they are accessing a resource that does not
> require authentication, they should omit the username from the URL).
It's tempting, because the non-netrc case is the common one, and we are
dropping the round-trip avoidance for those people. I'm just not sure
that it's going to be obvious to users that they need to drop the user@
portion from their URL when using netrc. That seems like a bizarre
requirement from the user's POV, even if we do document it.
-Peff
^ permalink raw reply
* Re: [PATCH v3 2/3] http.c: Use timeout suggested by curl instead of fixed 50ms timeout
From: Mika Fischer @ 2011-11-04 17:47 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, daniel, peff
In-Reply-To: <7vehxndd4q.fsf@alter.siamese.dyndns.org>
On Fri, Nov 4, 2011 at 18:13, Junio C Hamano <gitster@pobox.com> wrote:
> I'm inclined to squash in the following to narrow the scope of
> curl_timeout, though.
>
> diff --git a/http.c b/http.c
> index 5cb0fb6..924be52 100644
> --- a/http.c
> +++ b/http.c
> @@ -636,9 +636,6 @@ void run_active_slot(struct active_request_slot *slot)
> fd_set excfds;
> int max_fd;
> struct timeval select_timeout;
> -#if LIBCURL_VERSION_NUM >= 0x070f04
> - long curl_timeout;
> -#endif
> int finished = 0;
>
> slot->finished = &finished;
> @@ -655,6 +652,7 @@ void run_active_slot(struct active_request_slot *slot)
>
> if (slot->in_use && !data_received) {
> #if LIBCURL_VERSION_NUM >= 0x070f04
> + long curl_timeout;
> curl_multi_timeout(curlm, &curl_timeout);
> if (curl_timeout == 0) {
> continue;
Ah yes, that's good. I would have done it this way in C++, but I
wasn't sure whether C99 is OK for git.
^ permalink raw reply
* Re: [PATCH v3 2/3] http.c: Use timeout suggested by curl instead of fixed 50ms timeout
From: Jeff King @ 2011-11-04 17:51 UTC (permalink / raw)
To: Mika Fischer; +Cc: Junio C Hamano, git, daniel
In-Reply-To: <CAOs=hRKxc9SdE_HTnfs+WdnxZEY6yF9MBV_K1FX2=7B7xtj7-w@mail.gmail.com>
On Fri, Nov 04, 2011 at 06:47:44PM +0100, Mika Fischer wrote:
> > if (slot->in_use && !data_received) {
> > #if LIBCURL_VERSION_NUM >= 0x070f04
> > + long curl_timeout;
> > curl_multi_timeout(curlm, &curl_timeout);
> > if (curl_timeout == 0) {
> > continue;
>
> Ah yes, that's good. I would have done it this way in C++, but I
> wasn't sure whether C99 is OK for git.
C99 is not OK. But this is not C99, as the conditional opens a new
block.
-Peff
^ permalink raw reply
* Re: [PATCH v3 0/3] Improve use of select in http backend
From: Jeff King @ 2011-11-04 17:53 UTC (permalink / raw)
To: Mika Fischer; +Cc: git, gitster, daniel
In-Reply-To: <1320416367-28843-1-git-send-email-mika.fischer@zoopnet.de>
On Fri, Nov 04, 2011 at 03:19:24PM +0100, Mika Fischer wrote:
> Mika Fischer (3):
> http.c: Use curl_multi_fdset to select on curl fds instead of just
> sleeping
> http.c: Use timeout suggested by curl instead of fixed 50ms timeout
> http.c: Rely on select instead of tracking whether data was received
All three patches look good to me. Your 3/3 does most of the cleanup
from the other patch I posted, but we can also do this on top:
-- >8 --
Subject: [PATCH 4/3] http: drop "local" member from request struct
This is a FILE pointer in the case that we are sending our
output to a file. We originally used it to run ftell() to
determine whether data had been written to our file during
our last call to curl. However, as of the last patch, we no
longer care about that flag anymore. All uses of this struct
member are now just book-keeping that can go away.
Signed-off-by: Jeff King <peff@peff.net>
---
http.c | 6 ------
http.h | 1 -
2 files changed, 0 insertions(+), 7 deletions(-)
diff --git a/http.c b/http.c
index 3c6a00b..cfa9b07 100644
--- a/http.c
+++ b/http.c
@@ -535,7 +535,6 @@ struct active_request_slot *get_active_slot(void)
active_requests++;
slot->in_use = 1;
- slot->local = NULL;
slot->results = NULL;
slot->finished = NULL;
slot->callback_data = NULL;
@@ -829,7 +828,6 @@ static int http_request(const char *url, void *result, int target, int options)
headers = curl_slist_append(headers, buf.buf);
strbuf_reset(&buf);
}
- slot->local = result;
} else
curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION,
fwrite_buffer);
@@ -876,7 +874,6 @@ static int http_request(const char *url, void *result, int target, int options)
ret = HTTP_START_FAILED;
}
- slot->local = NULL;
curl_slist_free_all(headers);
strbuf_release(&buf);
@@ -1071,7 +1068,6 @@ void release_http_pack_request(struct http_pack_request *preq)
if (preq->packfile != NULL) {
fclose(preq->packfile);
preq->packfile = NULL;
- preq->slot->local = NULL;
}
if (preq->range_header != NULL) {
curl_slist_free_all(preq->range_header);
@@ -1093,7 +1089,6 @@ int finish_http_pack_request(struct http_pack_request *preq)
fclose(preq->packfile);
preq->packfile = NULL;
- preq->slot->local = NULL;
lst = preq->lst;
while (*lst != p)
@@ -1162,7 +1157,6 @@ struct http_pack_request *new_http_pack_request(
}
preq->slot = get_active_slot();
- preq->slot->local = preq->packfile;
curl_easy_setopt(preq->slot->curl, CURLOPT_FILE, preq->packfile);
curl_easy_setopt(preq->slot->curl, CURLOPT_WRITEFUNCTION, fwrite);
curl_easy_setopt(preq->slot->curl, CURLOPT_URL, preq->url);
diff --git a/http.h b/http.h
index 71bdf58..ee16069 100644
--- a/http.h
+++ b/http.h
@@ -49,7 +49,6 @@ struct slot_results {
struct active_request_slot {
CURL *curl;
- FILE *local;
int in_use;
CURLcode curl_result;
long http_code;
--
1.7.7.2.4.gfd7b9
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox