* [PATCH] Verify we know how to read a pack before trying to using it.
From: Shawn Pearce @ 2006-08-21 7:10 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
If the pack format were to ever change or be extended in the future
there is no assurance that just because the pack file lives in
objects/pack and doesn't end in .idx that we can read and decompress
its contents properly.
If we encounter what we think is a pack file and it isn't or we don't
recognize its version then die and suggest to the user that they
upgrade to a newer version of GIT which can handle that pack file.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
sha1_file.c | 12 ++++++++++++
1 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/sha1_file.c b/sha1_file.c
index d6eb9b3..bb9fa8b 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -481,6 +481,7 @@ int use_packed_git(struct packed_git *p)
int fd;
struct stat st;
void *map;
+ struct pack_header *hdr;
pack_mapped += p->pack_size;
while (PACK_MAX_SZ < pack_mapped && unuse_one_packed_git())
@@ -500,6 +501,17 @@ int use_packed_git(struct packed_git *p)
die("packfile %s cannot be mapped.", p->pack_name);
p->pack_base = map;
+ /* Check if we understand this pack file. If we don't we're
+ * likely too old to handle it.
+ */
+ hdr = map;
+ if (hdr->hdr_signature != htonl(PACK_SIGNATURE))
+ die("packfile %s isn't actually a pack.", p->pack_name);
+ if (!pack_version_ok(hdr->hdr_version))
+ die("packfile %s is version %i and not supported"
+ " (try upgrading GIT to a newer version)",
+ p->pack_name, ntohl(hdr->hdr_version));
+
/* Check if the pack file matches with the index file.
* this is cheap.
*/
--
1.4.2.g0dcb
^ permalink raw reply related
* Re: Huge win, compressing a window of delta runs as a unit
From: Shawn Pearce @ 2006-08-21 7:06 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Jon Smirl, git
In-Reply-To: <Pine.LNX.4.64.0608181057440.11359@localhost.localdomain>
Nicolas Pitre <nico@cam.org> wrote:
> On Fri, 18 Aug 2006, Jon Smirl wrote:
>
> > On 8/18/06, Nicolas Pitre <nico@cam.org> wrote:
> > > A better way to get such a size saving is to increase the window and
> > > depth parameters. For example, a window of 20 and depth of 20 can
> > > usually provide a pack size saving greater than 11% with none of the
> > > disadvantages mentioned above.
> >
> > Our window size is effectively infinite. I am handing him all of the
> > revisions from a single file in optimal order. This includes branches.
>
> In GIT packing terms this is infinite delta _depth_ not _window_.
We're not using infinite anything.
fast-import is basically doing window=1 and depth=10.
We only examine the last blob to see if we can get a delta against
it. If we do we write that delta out; otherwise we reset our delta
chain and write the complete object. We also reset our chain after
writing out 10 deltas, each of which used the immediately prior
object as its base.
Since I just found out that in some cases the Mozilla repository has
1000s of revisions per file[*1*] and in others only 1 revision per
file we probably should be adjusting this depth to have a maximum
of 500 while also having the frontend send us a "I'm switching
files now" marker so we know to not even bother trying to delta
the new blob against the last blob as they are likely to not
delta well[*2*].
> Default delta params (window=10 depth=10) : 122103455
> Agressive deltas (window=50 depth=5000) : 105870516
> Agressive and grouped deltas (window=50 depth=5000 : 99860685
Although complex the aggressive and grouped deltas appears to
have saved you 18.2% on this repository. That's not something
to ignore. A reasonably optimal local pack dictionary could save
at least 4%[*3*]. Whacking 22% off a 400 MB pack is saving 88 MB.
Transferring that over the network on an initial clone is like
downloading all of Eclipse. Or an uncompressed kernel tarball...
[*1*] Jon noted this in another email in this thread but I'm too
lazy to lookup the hyperlink right now.
[*2*] Granted in some cases they may delta very well against each
other but I think the probablity of that occuring is low
enough that its not worth worrying about in fast-import.c;
we can let repack's strategy deal with it instead.
[*3*] I wrote a brain-dead simple local dictionary selecter in Perl.
Its horribly far from being ideal. But it is consistently
saving us 4% on the GIT and the Mozilla repository and its
pretty darn fast. Shockingly the C keywords didn't gain
us very much here; its project specific text that's the
real win.
Looking at chunks which are frequently copied in deltas
from base objects and breaking those chunks up into
smaller common chunks, then loading those most frequent
common chunks into the pack dictionary would most likely
produce far better results.
--
Shawn.
^ permalink raw reply
* Re: Huge win, compressing a window of delta runs as a unit
From: Shawn Pearce @ 2006-08-21 6:46 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Jon Smirl, git
In-Reply-To: <Pine.LNX.4.64.0608202257020.3682@localhost.localdomain>
Nicolas Pitre <nico@cam.org> wrote:
> I looked at it too and the code looks OK.
Based on the stats Jon provided me it appeared as though the code
skipped about 9% of the objects, and at the time I thought I new
why but looking at it again right now I'm not sure why. :-)
Assuming the objects were roughly the same size (probably a bad
assumption, but lets use it anyway) we're talking about approx. 9%
increase, bringing the size to around 360 MB.
Since that is still only blobs and lacks trees&commits, and you were
able to get a pack from the earlier import attempts down to 430 MB
(including trees&commits) everything you pointed out with regards
to delta base selection not being optimal is probably the major
culprit here.
> So I think that trying to make fastimport too clever is wrong. It
> should instead focus on creating an initial pack as fast as possible and
> then rely on a final git-repack pass to produce the shrinked pack. I
> really doubt the import code could ever make a better job than
> git-pack-objects does.
I'm mixed on that.
My goal with fast-import was to construct a pack as cheaply as
possible in such a way that IO is relatively minimized during
pack construction and later during an initial repacking, as well
as to avoid all fork'ing costs associated with many update-index
and repack calls.
fast-import is assuming the front end driving it is being somewhat
nice by feeding revisions in newest-oldest order, one file at a time.
If this assumption is false it will produce a pack but one that is
so large the IO to repack it will be horrible (but probably would
still be better than if these were all loose objects).
fast-import is also assuming that pack construction speed and memory
usage required to build that pack are more important than subsequent
reads of the resulting pack.
Why?
Because I fully expected the pack to undergo a `git-repack -a -d -f`
with a rather large delta depth before it really got used by
clients which cared about performance or disk space. :-)
Whether or not git-repack can handle such a large pack is another
story. Apparently it can already take a ~600 MB pack and do
something useful to it in a reasonable time span so I'm putting
any limits that might be in there off for now.
But the more I work with Jon on this Mozilla import process the more
I'm realizing that:
- fast-import.c's other name is "GIT in 1325 lines of C code, all
of which more or less already exists elsewhere in a form that
wasn't useful in this context without refactoring it in major
ways so I got lazy and rewrote it in another equally useless way";
- the strategy of how I'm slamming a very large number of objects
into a pack may be useful in situations other than a foreign
SCM import process. I can see someone wanting to create a
large commit with a lot of modified objects. Doing this with
update-index and write-tree into loose objects would be far
slower than just generating a new pack if the number of objects
you are writing exceeds about 100 on Windows or ~1k on UNIX;
- Jon has lots of CPU time to spare and possibly even some memory.
fast-import is barely using CPU and barely uses memory (its quite
lightweight!) but its definately producing a suboptimal pack
as a result. Just doing slightly better delta selection before
committing an object to the output pack may prevent the need to
use -f during git-repack and produce something almost as optimal.
Right now fast-import is only lacking the following major features,
at which point it *should* be able to process the entire Mozilla
repository, assuming Jon can get his symbol closing problem in
cvs2svn solved:
- reload a branch which has been swapped out (requires reading
the pack that is currently being constructed back into memory,
which since its not really a valid pack yet and lacks an index
reusing the code in sha1_file.c is slightly interesting);
- generate a tag object into the pack;
- track association ID numbers to SHA1 IDs (needed to properly
create a branch from a prior commit or a tag from a prior commit
as the prior commit's SHA ID isn't known to the front end);
Working on the multiple objects in one zlib stream code over the
weekend also showed me how to use sha1_file.c to perform the first
of the three (I think) and the remaining two are trivial. So my
weekend wasn't a total loss. :-)
I plan on getting that code finished up in the next day or so and
get a new draft of it off to Jon. Maybe by then he'll also have
something coming out of cvs2svn's pass 9.
> > As comparison, I just tar/zipped the Mozilla CVS repo and it is 541MB.
> > The 295MB git pack number does not have commits and trees in it, it is
> > revisions only.
>
> Running git-repack -a -f from a recent GIT on the Mozilla repo converted
> through cvsps and friends produces a pack smaller than 500MB. I even
> brought it down to 430MB by using non default delta window and depth.
I'd still like to find a way to beat that 430 MB current
best-yet-still-valid size. :-)
I think we'll get there. Shaving off 4-5% with a pack specific
dictionary may be worthwhile on such large packs, especially ones
that are considered "historical" and probably will never get repacked
again. Folding objects into a single zlib stream may also be worth
it, but for now I'm going to wait and see what mileage we can get
from the existing repack code and the existing pack file format.
--
Shawn.
^ permalink raw reply
* Re: [PATCH] Helper function to shell quote all arg values at once.
From: Christian Couder @ 2006-08-21 5:15 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vsljrxbg8.fsf@assigned-by-dhcp.cox.net>
Le lundi 21 août 2006 00:57, Junio C Hamano a écrit :
> Christian Couder <chriscool@tuxfamily.org> writes:
>
> > The new sq_quote_argv function is used to refactor the
> > tracing code in "git.c" and "exec_cmd.c".
> > This function allocates memory and fills it with a string
> > containing the quoted argument values. Then it returns a
> > pointer to this memory that must be freed afterwards.
>
> Sorry, I do not see a point in this.
>
> If your original were doing malloc-print-free per iteration,
> then perhaps it makes sense to first format all into one
> allocated buffer, print all, and then free at once, like this
> patch does. But that was not what the original had.
I thought that malloc-print-free was Ok, since there is already sq_quote
in quote.c that does it and is used in connect.c (except perhaps that the result from sq_quote is not freed).
> If the new function were to get a (const char **) array and
> FILE *, and print them, quoted and separated with spaces, then
> it would have shortened what two call sites did, which would
> have been an improvement. But that is not what this patch does,
> either.
The patch still shortens the 3 calls site in git.c (-8 lines) and exec_cmd.c (-5 lines).
But anyway I will rewrite it so that the new function takes (FILE*, const char **, int count) as argument.
Or perhaps I need only make this new function call sq_quote_argv ? So I only need to send another patch on top of this one ?
Thanks,
Christian.
^ permalink raw reply
* Feel Pleasure from Take control over your male body image. This delivers amazing results and makes your dearest limb bigger and better-working!
From: Lazaro @ 2006-08-21 4:47 UTC (permalink / raw)
To: git
New’n’hot
Finally there is something that really gives you extra inches and extra confidence. All you need is a supply for a month or so! She will beg you to order more after you use it for a month.
Stop taking that size-does-not-matter crap from girls! They just don't want to hurt you! Try to imagine your totally new life free from size fears, lack of confidence and with terrific success among females. Come on in: http://www.dfegi.com/gall/ms/
Imagine that this is bigger and moreover, stays big forever!
People who think the world revolves around them have no sense of direction Like the measles, love is most dangerous when it comes late in life When you become successful in any type of life, there are people who are not contributing to the motion
^ permalink raw reply
* Re: Unresolved issues #3
From: A Large Angry SCM @ 2006-08-21 4:05 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Nicolas Pitre
In-Reply-To: <7vk653xa3a.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano wrote:
> Nicolas Pitre <nico@cam.org> writes:
>
>> On Fri, 18 Aug 2006, A Large Angry SCM wrote:
>>
>>> Historic fact. Between Thu May 19 08:56:22 2005 and Thu Feb 9 21:06:38
>>> 2006 bit 6 of the first byte of a delta hunk was interpreted to mean
>>> that the source of the copy was the result buffer. From Thu May 19
>>> 08:56:22 2005 on, the code to decode delta hunks in type 2 packs was
>>> available to everyone and anyone interested could make a pack encoder
>>> that would create packs that the core Git code would correctly read. The
>>> commit of Thu Feb 9 21:06:38 2006, d60fc, actually introduced a bug
>>> that would treat valid type 2 packs as invalid.
>
> It is more like the said commit made the pack format extensible
> by declaring the bit reserved for the future use, by declaring
> retroactively that a type 2 pack that used that bit invalid.
> And it was deemed a reasonable and safe decision because no
> official git ever produced a type 2 pack that used that bit,
>
> Yes, that was a backward incompatible change, strictly speaking,
> and probably I should have made an announcement that looked
> similar to this by Linus:
>
> From: Linus Torvalds <torvalds@osdl.org>
> Subject: CAREFUL! No more delta object support!
> Date: Mon, 27 Jun 2005 18:14:40 -0700 (PDT)
> Message-ID: <Pine.LNX.4.58.0506271755140.19755@ppc970.osdl.org>
> To: Git Mailing List <git@vger.kernel.org>
>
> So you could argue I was incompetent not to make a big fuss
> about this backward incompatibility back then, if you like.
>
> I did not think it was worth it back then, and I do not think it
> is worth it now, either. But if it makes you feel better, I
> could retroactively make such an announcement about the
> unofficial bit 6.
>
> The announcement would have read like this:
>
> The current git code does not support type #2 packs that
> uses delta with bit 6 to mean "copy inside destination
> buffer". Although the code that interpreted delta data
> supported bit 6 that way for a brief period of time, no
> official git ever released produced delta that used the
> bit that way.
>
> In other words, if you have created packs with your own,
> modified git, that took advantage of "copy inside
> destination buffer" feature in the delta interpretation
> code, such packs are not usable by the official git, so
> you need to unpack them using your own version of git
> and then repack with the official version of git.
Please read the commit message for commit d60fc. It's type _3_ pack
files that redefined bit 6 to add the extra byte of copy length, not
type 2. Thus, no need to retroactively invalidate the type 2 pack files
that used copy from result.
^ permalink raw reply
* Re: Huge win, compressing a window of delta runs as a unit
From: Nicolas Pitre @ 2006-08-21 3:45 UTC (permalink / raw)
To: Jon Smirl; +Cc: Shawn Pearce, git
In-Reply-To: <9e4733910608180956n64e3362fm5c72d652e6b6243a@mail.gmail.com>
On Fri, 18 Aug 2006, Jon Smirl wrote:
> On 8/18/06, Nicolas Pitre <nico@cam.org> wrote:
> > On Fri, 18 Aug 2006, Jon Smirl wrote:
> >
> > > I attached Shawn's code. He is gone until Monday and can't defend it.
> >
> > I will have a look at it next week as I'll be gone for the weekend as
> > well.
>
> I looked at it some and couldn't see anything obviously wrong with it,
> but it wasn't a detailed inspection.
I looked at it too and the code looks OK.
This doesn't mean there is no problem at a higher level though. The
deltification process is extremely crude and I think this is the cause
of the original pack size.
For example, last April we discovered that a small change in the
heuristics to determine base delta objects in git-pack-objects could
create a pack size regression up to 4x the size of the same pack created
before such change.
It is also possible to have a denser delta stream but once deflated it
is larger than a less dense delta to start with.
Just to say that many tweaks and heuristics have been implemented and
studied in git-pack-objects for over a year now in order to get the
really small packs we have today. And a really subtle and
inocent-looking change can break it size wize.
So what I think is happening with the fastimport code is that the delta
selection is not really good. It is certainly much better than no delta
at all but still not optimal which smells deja vu to me. Then by
deflating them all together the redundent information that the bad delta
set still carries along is eliminated -- thanks to zlib sort of
mitigating the real issue.
But... as my recent experiments show, the grouping of related deltas
into a single zlib stream doesn't produce significant improvements when
implemented directly into git-pack-objects. Certainly not worth the
inconvenients and costs it brings along. I even think that if you used
git-repack -a -f on the pack produced by the import process, with only
delta deflated individually just like it did originally, then the
repacked pack would _also_ shrink significantly. Most probably around
4x just like you observed with the grouping of deltas in the same zlib
stream.
Not only would git-repack make it much smaller, but it also provicdes a
much better layout where all objects for recent commits are all stored
together at the beginning of the pack. The fastimport code is instead
storing them scattered all over the pack for every commit by making all
revisions of each file next to each other which will cause horrible
access patterns and really bad IO.
So I think that trying to make fastimport too clever is wrong. It
should instead focus on creating an initial pack as fast as possible and
then rely on a final git-repack pass to produce the shrinked pack. I
really doubt the import code could ever make a better job than
git-pack-objects does.
If I can make a suggestion, you should forget about this multiple deltas
in one zlib stream for now and focus on making the import process work
all the way to tree and commit objects instead. Then, only then, if
git-repack -a -f doesn't produce satisfactory pack size we could look at
better pack encoding. And so far the grouping of related deltas in one
zlib stream is _not_ a better encoding given the rather small
improvement over unmodified git-pack-objects vs the inconvenients and
cost it brings with it.
> As comparison, I just tar/zipped the Mozilla CVS repo and it is 541MB.
> The 295MB git pack number does not have commits and trees in it, it is
> revisions only.
Running git-repack -a -f from a recent GIT on the Mozilla repo converted
through cvsps and friends produces a pack smaller than 500MB. I even
brought it down to 430MB by using non default delta window and depth.
Nicolas
^ permalink raw reply
* [PATCH] daemon: add upload-tar service.
From: Junio C Hamano @ 2006-08-21 2:09 UTC (permalink / raw)
To: git
In-Reply-To: <7vy7tjxdas.fsf@assigned-by-dhcp.cox.net>
This allows clients to ask for tarballs with:
git tar-tree --remote=git://server/repo refname
By default, the upload-tar service is not enabled. To enable
it server-wide, the server can be started with:
git-daemon --enable=upload-tar
This service is by default overridable per repostiory, so
alternatively, a repository can define "daemon.uploadtar = true"
to enable it.
Signed-off-by: Junio C Hamano <junkio@cox.net>
---
daemon.c | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/daemon.c b/daemon.c
index 0a6cbb4..5549014 100644
--- a/daemon.c
+++ b/daemon.c
@@ -321,8 +321,15 @@ static int upload_pack(void)
return -1;
}
+static int upload_tar(void)
+{
+ execl_git_cmd("upload-tar", ".", NULL);
+ return -1;
+}
+
static struct daemon_service daemon_service[] = {
{ "upload-pack", "uploadpack", upload_pack, 1, 1 },
+ { "upload-tar", "uploadtar", upload_tar, 0, 1 },
};
static void enable_service(const char *name, int ena) {
--
1.4.2.g0cac8
^ permalink raw reply related
* [PATCH] daemon: prepare for multiple services.
From: Junio C Hamano @ 2006-08-21 2:09 UTC (permalink / raw)
To: git
In-Reply-To: <7vy7tjxdas.fsf@assigned-by-dhcp.cox.net>
This adds an infrastructure to selectively enable and disable
more than one services in git-daemon. Currently upload-pack
service, which serves the git-fetch-pack and git-peek-remote
clients, is the only service that is defined.
Signed-off-by: Junio C Hamano <junkio@cox.net>
---
daemon.c | 113 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----
1 files changed, 106 insertions(+), 7 deletions(-)
diff --git a/daemon.c b/daemon.c
index 012936f..0a6cbb4 100644
--- a/daemon.c
+++ b/daemon.c
@@ -229,13 +229,42 @@ static char *path_ok(char *dir)
return NULL; /* Fallthrough. Deny by default */
}
-static int upload(char *dir)
+typedef int (*daemon_service_fn)(void);
+struct daemon_service {
+ const char *name;
+ const char *config_name;
+ daemon_service_fn fn;
+ int enabled;
+ int overridable;
+};
+
+static struct daemon_service *service_looking_at;
+static int service_enabled;
+
+static int git_daemon_config(const char *var, const char *value)
+{
+ if (!strncmp(var, "daemon.", 7) &&
+ !strcmp(var + 7, service_looking_at->config_name)) {
+ service_enabled = git_config_bool(var, value);
+ return 0;
+ }
+
+ /* we are not interested in parsing any other configuration here */
+ return 0;
+}
+
+static int run_service(char *dir, struct daemon_service *service)
{
- /* Timeout as string */
- char timeout_buf[64];
const char *path;
+ int enabled = service->enabled;
+
+ loginfo("Request %s for '%s'", service->name, dir);
- loginfo("Request for '%s'", dir);
+ if (!enabled && !service->overridable) {
+ logerror("'%s': service not enabled.", service->name);
+ errno = EACCES;
+ return -1;
+ }
if (!(path = path_ok(dir)))
return -1;
@@ -257,12 +286,34 @@ static int upload(char *dir)
return -1;
}
+ if (service->overridable) {
+ service_looking_at = service;
+ service_enabled = -1;
+ git_config(git_daemon_config);
+ if (0 <= service_enabled)
+ enabled = service_enabled;
+ }
+ if (!enabled) {
+ logerror("'%s': service not enabled for '%s'",
+ service->name, path);
+ errno = EACCES;
+ return -1;
+ }
+
/*
* We'll ignore SIGTERM from now on, we have a
* good client.
*/
signal(SIGTERM, SIG_IGN);
+ return service->fn();
+}
+
+static int upload_pack(void)
+{
+ /* Timeout as string */
+ char timeout_buf[64];
+
snprintf(timeout_buf, sizeof timeout_buf, "--timeout=%u", timeout);
/* git-upload-pack only ever reads stuff, so this is safe */
@@ -270,10 +321,36 @@ static int upload(char *dir)
return -1;
}
+static struct daemon_service daemon_service[] = {
+ { "upload-pack", "uploadpack", upload_pack, 1, 1 },
+};
+
+static void enable_service(const char *name, int ena) {
+ int i;
+ for (i = 0; i < ARRAY_SIZE(daemon_service); i++) {
+ if (!strcmp(daemon_service[i].name, name)) {
+ daemon_service[i].enabled = ena;
+ return;
+ }
+ }
+ die("No such service %s", name);
+}
+
+static void make_service_overridable(const char *name, int ena) {
+ int i;
+ for (i = 0; i < ARRAY_SIZE(daemon_service); i++) {
+ if (!strcmp(daemon_service[i].name, name)) {
+ daemon_service[i].overridable = ena;
+ return;
+ }
+ }
+ die("No such service %s", name);
+}
+
static int execute(struct sockaddr *addr)
{
static char line[1000];
- int pktlen, len;
+ int pktlen, len, i;
if (addr) {
char addrbuf[256] = "";
@@ -310,8 +387,14 @@ #endif
if (len && line[len-1] == '\n')
line[--len] = 0;
- if (!strncmp("git-upload-pack ", line, 16))
- return upload(line+16);
+ for (i = 0; i < ARRAY_SIZE(daemon_service); i++) {
+ struct daemon_service *s = &(daemon_service[i]);
+ int namelen = strlen(s->name);
+ if (!strncmp("git-", line, 4) &&
+ !strncmp(s->name, line + 4, namelen) &&
+ line[namelen + 4] == ' ')
+ return run_service(line + namelen + 5, s);
+ }
logerror("Protocol error: '%s'", line);
return -1;
@@ -791,6 +874,22 @@ int main(int argc, char **argv)
log_syslog = 1;
continue;
}
+ if (!strncmp(arg, "--enable=", 9)) {
+ enable_service(arg + 9, 1);
+ continue;
+ }
+ if (!strncmp(arg, "--disable=", 10)) {
+ enable_service(arg + 10, 0);
+ continue;
+ }
+ if (!strncmp(arg, "--enable-override=", 18)) {
+ make_service_overridable(arg + 18, 1);
+ continue;
+ }
+ if (!strncmp(arg, "--disable-override=", 19)) {
+ make_service_overridable(arg + 19, 0);
+ continue;
+ }
if (!strcmp(arg, "--")) {
ok_paths = &argv[i+1];
break;
--
1.4.2.g0cac8
^ permalink raw reply related
* Re: [PATCH] branch as a builtin (again)
From: Johannes Schindelin @ 2006-08-20 23:55 UTC (permalink / raw)
To: Kristian Høgsberg; +Cc: git
In-Reply-To: <59ad55d30608201422h4a6d40f7y7782212637380438@mail.gmail.com>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 760 bytes --]
Hi,
On Sun, 20 Aug 2006, Kristian Høgsberg wrote:
> Hi,
>
> I sent a patch to rewrite branch in C and make it a builtin a couple
> of months ago. Junio had a few comments about the patch that I now
> finally had the time to address. One of the problems was that
> merge-base didn't clean up its state, which has now been fixed.
> Here's the updated version.
>
> Signed-off-by: Kristian Høgsberg <krh@redhat.com>
Micro-nit: in delete_branches(), you can assign rev2 outside the loop.
Also, "git branch" no longer sorts the names alphabetically (you could fix
this by putting the names into a path_list, which sorts them).
The "-l" option (create a log) seems to be forgotten, but then, I never
use that anyway.
AFAICT the rest is fine.
Ciao,
Dscho
^ permalink raw reply
* Re: [ANNOUNCE] git-rev-size: calculate sizes of repository
From: Johannes Schindelin @ 2006-08-20 23:36 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Rutger Nijlunsing
In-Reply-To: <7vlkpjytnj.fsf@assigned-by-dhcp.cox.net>
Hi,
On Sun, 20 Aug 2006, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > On Sun, 20 Aug 2006, Rutger Nijlunsing wrote:
> >
> >> I _knew_ it... Please go bug someone else. The only thing I did was
> >> help someone, and for that I choose my own tools since I do it for
> >> fun.
> >
> > Fair enough.
> >
> > -- 8< --
> > [PATCH] Add git-rev-size
> >
> > This tool spits out the number of trees, the number of blobs, and the total
> > bytes of the blobs for a given rev range.
>
> I do not speak ruby (well I suspect I could read it if I wanted
> to but I didn't try) so this may or may not be something
> Johannes inherited from the original,
No, it was no rewrite. But looking at the Ruby code again, it is not
really similar: the builtin uses the hash to cache the sizes even for a
blob. Further, it does not unpack the objects (except for the trees, and
for the revision walk if you limit by pathname). However, it inherits
this:
> but I think the code overcounts blobs and trees for a top-level tree
> that happens to have the same blob (or tree) twice. I am not sure if
> that is intended.
>
> Overcounting would give closer estimate for how big a tar
> archive would be, or how big an populated working tree would be,
> so it could be considered a feature. It all depends on what
> this tools is useful for, I guess.
I dunno. No idea what the original requester wanted to do with it.
For me, it was a nice distraction from my work. And a nice occasion to
finally copy^H^H^H^Himplement the independent hash map code I always
wanted to refactor from object.c. And a nice demonstration how easy it
actually is these days to implement a builtin.
Ciao,
Dscho
^ permalink raw reply
* Re: Unresolved issues #3
From: Junio C Hamano @ 2006-08-20 23:26 UTC (permalink / raw)
To: A Large Angry SCM; +Cc: git, Nicolas Pitre
In-Reply-To: <Pine.LNX.4.64.0608201840310.11359@localhost.localdomain>
Nicolas Pitre <nico@cam.org> writes:
> On Fri, 18 Aug 2006, A Large Angry SCM wrote:
>
>> Historic fact. Between Thu May 19 08:56:22 2005 and Thu Feb 9 21:06:38
>> 2006 bit 6 of the first byte of a delta hunk was interpreted to mean
>> that the source of the copy was the result buffer. From Thu May 19
>> 08:56:22 2005 on, the code to decode delta hunks in type 2 packs was
>> available to everyone and anyone interested could make a pack encoder
>> that would create packs that the core Git code would correctly read. The
>> commit of Thu Feb 9 21:06:38 2006, d60fc, actually introduced a bug
>> that would treat valid type 2 packs as invalid.
It is more like the said commit made the pack format extensible
by declaring the bit reserved for the future use, by declaring
retroactively that a type 2 pack that used that bit invalid.
And it was deemed a reasonable and safe decision because no
official git ever produced a type 2 pack that used that bit,
Yes, that was a backward incompatible change, strictly speaking,
and probably I should have made an announcement that looked
similar to this by Linus:
From: Linus Torvalds <torvalds@osdl.org>
Subject: CAREFUL! No more delta object support!
Date: Mon, 27 Jun 2005 18:14:40 -0700 (PDT)
Message-ID: <Pine.LNX.4.58.0506271755140.19755@ppc970.osdl.org>
To: Git Mailing List <git@vger.kernel.org>
So you could argue I was incompetent not to make a big fuss
about this backward incompatibility back then, if you like.
I did not think it was worth it back then, and I do not think it
is worth it now, either. But if it makes you feel better, I
could retroactively make such an announcement about the
unofficial bit 6.
The announcement would have read like this:
The current git code does not support type #2 packs that
uses delta with bit 6 to mean "copy inside destination
buffer". Although the code that interpreted delta data
supported bit 6 that way for a brief period of time, no
official git ever released produced delta that used the
bit that way.
In other words, if you have created packs with your own,
modified git, that took advantage of "copy inside
destination buffer" feature in the delta interpretation
code, such packs are not usable by the official git, so
you need to unpack them using your own version of git
and then repack with the official version of git.
^ permalink raw reply
* Re: Unresolved issues #3
From: Nicolas Pitre @ 2006-08-20 23:10 UTC (permalink / raw)
To: A Large Angry SCM; +Cc: Junio C Hamano, git
In-Reply-To: <44E68DCD.8010603@gmail.com>
On Fri, 18 Aug 2006, A Large Angry SCM wrote:
> Nicolas Pitre wrote:
> > On Fri, 18 Aug 2006, A Large Angry SCM wrote:
> >> That doesn't prove the non-existence of other code to do it.
> >
> > So? If the official and primary code for GIT doesn't support it, what
> > is the point? I'm telling you that if such packs exist they will simply
> > barf with all official GIT releases later than v1.1.6 making your
> > argument pointless.
> >
> > I don't mind you documenting that historic intent for a bit that was
> > never officially used, but at least let's document it right.
>
> Historic fact. Between Thu May 19 08:56:22 2005 and Thu Feb 9 21:06:38
> 2006 bit 6 of the first byte of a delta hunk was interpreted to mean
> that the source of the copy was the result buffer. From Thu May 19
> 08:56:22 2005 on, the code to decode delta hunks in type 2 packs was
> available to everyone and anyone interested could make a pack encoder
> that would create packs that the core Git code would correctly read. The
> commit of Thu Feb 9 21:06:38 2006, d60fc, actually introduced a bug
> that would treat valid type 2 packs as invalid.
The "actually introduced a bug" sentence is your own interpretation not
a _fact_.
And I simply disagree with that interpretation of yours.
I don't think this is worth arguing any further.
Nicolas
^ permalink raw reply
* Re: [PATCH] Helper function to shell quote all arg values at once.
From: Junio C Hamano @ 2006-08-20 22:57 UTC (permalink / raw)
To: Christian Couder; +Cc: git
In-Reply-To: <20060820080749.e31276dd.chriscool@tuxfamily.org>
Christian Couder <chriscool@tuxfamily.org> writes:
> The new sq_quote_argv function is used to refactor the
> tracing code in "git.c" and "exec_cmd.c".
> This function allocates memory and fills it with a string
> containing the quoted argument values. Then it returns a
> pointer to this memory that must be freed afterwards.
Sorry, I do not see a point in this.
If your original were doing malloc-print-free per iteration,
then perhaps it makes sense to first format all into one
allocated buffer, print all, and then free at once, like this
patch does. But that was not what the original had.
If the new function were to get a (const char **) array and
FILE *, and print them, quoted and separated with spaces, then
it would have shortened what two call sites did, which would
have been an improvement. But that is not what this patch does,
either.
^ permalink raw reply
* Re: Unresolved issues #3
From: Junio C Hamano @ 2006-08-20 22:17 UTC (permalink / raw)
To: Jon Loeliger; +Cc: git
In-Reply-To: <1155923876.10054.205.camel@cashmere.sps.mot.com>
Jon Loeliger <jdl@freescale.com> writes:
> I have another:
>
> git-daemon virtualization so that consistent HTTP and
> native git protocols can appear to use consistent URLs
> even in the face of HTTP configurations aliasing them
> to somewhere else on the filesystem and for multiple
> virtually hosted domain names.
Good point.
Here are a handful others I have about git-daemon:
* Possibly add --strict-symlink option we discussed a long time
ago that prevents symbolic links inside the named whitelisted
directory hierarchies to step outside.
Nobody actually jumped up-and-down asking for it since we
initially discussed this, so we may not need it after all.
* The whitelist directories need to be specified with full path
even when --base-path is used, which somewhat felt wrong. I
do not have strong feeling about this anymore, though.
* Extend $GIT_DIR/git-daemon-export-ok mechanism and have it
read from $GIT_DIR/config.
* Also extend the same in a way similar to the discussion we
had between Aneesh, me and Jakub to control 'blame' and
'snapshot' in gitweb. I.e. allow the site-wide configuration
to specify which services are enabled by default and which
services can be overridden per repository, and allow per
repository configuration to specify what's enabled and what's
not.
* Add git-upload-tar as a potential program that can be
executed from git-daemon.
If we are to do the last three items, my thinking is to do
something like this:
* The config file can have:
daemon.upload-pack = yes | no
daemon.upload-tar = yes | no
to enable or disable the services individually.
* $GIT_DIR/git-daemon-export-ok is kept as a backward
compatibility measure; it means upload-pack is enabled.
If we were to allow git-upload-tar from git-daemon:
* Fix protocol exchange between git-upload-tar and git-tar-tree
so that the downloader can name the <tree-ish> symbolically
like this:
git tar-tree --remote=git://git.kernel.org/pub/scm/git/git next
Currently the downloader has to get the value of "next" at
the remote server out-of-band and give an explicit object
name.
^ permalink raw reply
* Re: [ANNOUNCE] git-rev-size: calculate sizes of repository
From: Junio C Hamano @ 2006-08-20 21:38 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Rutger Nijlunsing
In-Reply-To: <Pine.LNX.4.63.0608201805070.28360@wbgn013.biozentrum.uni-wuerzburg.de>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> On Sun, 20 Aug 2006, Rutger Nijlunsing wrote:
>
>> I _knew_ it... Please go bug someone else. The only thing I did was
>> help someone, and for that I choose my own tools since I do it for
>> fun.
>
> Fair enough.
>
> -- 8< --
> [PATCH] Add git-rev-size
>
> This tool spits out the number of trees, the number of blobs, and the total
> bytes of the blobs for a given rev range.
I do not speak ruby (well I suspect I could read it if I wanted
to but I didn't try) so this may or may not be something
Johannes inherited from the original, but I think the code
overcounts blobs and trees for a top-level tree that happens to
have the same blob (or tree) twice. I am not sure if that is
intended.
Overcounting would give closer estimate for how big a tar
archive would be, or how big an populated working tree would be,
so it could be considered a feature. It all depends on what
this tools is useful for, I guess.
^ permalink raw reply
* [PATCH] branch as a builtin (again)
From: Kristian Høgsberg @ 2006-08-20 21:22 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: text/plain, Size: 349 bytes --]
Hi,
I sent a patch to rewrite branch in C and make it a builtin a couple
of months ago. Junio had a few comments about the patch that I now
finally had the time to address. One of the problems was that
merge-base didn't clean up its state, which has now been fixed.
Here's the updated version.
Signed-off-by: Kristian Høgsberg <krh@redhat.com>
[-- Attachment #2: builtin-branch.patch --]
[-- Type: application/octet-stream, Size: 9011 bytes --]
commit 8153d336ad9643507922932a2ac2277073c66432
Author: Kristian Høgsberg <krh@redhat.com>
Date: Sun Aug 20 17:04:14 2006 -0400
Rewrite branch in C and make it a builtin.
diff --git a/Makefile b/Makefile
index 23cd8a0..adf043e 100644
--- a/Makefile
+++ b/Makefile
@@ -149,7 +149,7 @@ SPARSE_FLAGS = -D__BIG_ENDIAN__ -D__powe
### --- END CONFIGURATION SECTION ---
SCRIPT_SH = \
- git-bisect.sh git-branch.sh git-checkout.sh \
+ git-bisect.sh git-checkout.sh \
git-cherry.sh git-clean.sh git-clone.sh git-commit.sh \
git-fetch.sh \
git-ls-remote.sh \
@@ -253,6 +253,7 @@ LIB_OBJS = \
BUILTIN_OBJS = \
builtin-add.o \
builtin-apply.o \
+ builtin-branch.o \
builtin-cat-file.o \
builtin-checkout-index.o \
builtin-check-ref-format.o \
diff --git a/builtin-branch.c b/builtin-branch.c
new file mode 100644
index 0000000..25c6313
--- /dev/null
+++ b/builtin-branch.c
@@ -0,0 +1,170 @@
+/*
+ * Builtin "git branch"
+ *
+ * Copyright (c) 2006 Kristian Høgsberg <krh@redhat.com>
+ * Based on git-branch.sh by Junio C Hamano.
+ */
+
+#include "cache.h"
+#include "refs.h"
+#include "commit.h"
+#include "builtin.h"
+
+static const char builtin_branch_usage[] =
+ "git-branch [(-d | -D) <branchname>] | [[-f] <branchname> [<start-point>]] | -r";
+
+
+static int remote_only = 0;
+static const char *head;
+static unsigned char head_sha1[20];
+
+static int in_merge_bases(const unsigned char *sha1,
+ struct commit *rev1,
+ struct commit *rev2)
+{
+ struct commit_list *bases, *b;
+
+ bases = get_merge_bases(rev1, rev2, 1);
+ for (b = bases; b != NULL; b = b->next) {
+ if (!hashcmp(sha1, b->item->object.sha1)) {
+ free_commit_list(bases);
+ return 1;
+ }
+ }
+
+ free_commit_list(bases);
+ return 0;
+}
+
+static void delete_branches(int argc, const char **argv, int force)
+{
+ struct commit *rev1, *rev2;
+ unsigned char sha1[20];
+ const char *p, *name;
+ int i;
+
+ for (i = 0; i < argc; i++) {
+ if (!strcmp(head, argv[i]))
+ die("Cannot delete the branch you are currently on.");
+
+ name = git_path("refs/heads/%s", argv[i]);
+ p = resolve_ref(name, sha1, 1);
+ if (p == NULL)
+ die("Branch '%s' not found.", argv[i]);
+
+ rev1 = lookup_commit_reference(sha1);
+ rev2 = lookup_commit_reference(head_sha1);
+ if (!rev1 || !rev2)
+ die("Couldn't look up commit objects.");
+
+ /* This checks wether the merge bases of branch and
+ * HEAD contains branch -- which means that the HEAD
+ * contains everything in both.
+ */
+
+ if (!force &&
+ !in_merge_bases(sha1, rev1, rev2)) {
+ fprintf(stderr,
+ "The branch '%s' is not a strict subset of your current HEAD.\n"
+ "If you are sure you want to delete it, run 'git branch -D %s'.\n",
+ argv[i], argv[i]);
+ exit(1);
+ }
+
+ unlink(name);
+ printf("Deleted branch %s.\n", argv[i]);
+ }
+}
+
+static int show_reference(const char *refname, const unsigned char *sha1)
+{
+ int is_head = !strcmp(refname, head);
+
+ printf("%c %s\n", (is_head ? '*' : ' '), refname);
+
+ return 0;
+}
+
+static void create_branch (const char *name, const char *start, int force)
+{
+ struct ref_lock *lock;
+ unsigned char sha1[20];
+ char ref[PATH_MAX];
+
+ snprintf(ref, sizeof ref, "refs/heads/%s", name);
+ if (check_ref_format(ref + 5))
+ die("'%s' is not a valid branch name.", name);
+
+ if (resolve_ref(ref, sha1, 1)) {
+ if (!force)
+ die("A branch named '%s' already exists.", name);
+ else if (!strcmp(head, name))
+ die("Cannot force update the current branch.");
+ }
+
+ if (get_sha1(start, sha1))
+ die("Not a valid branch point: '%s'", start);
+
+ lock = lock_any_ref_for_update(ref, NULL, 0);
+ if (!lock)
+ die("Failed to lock ref for update: %s.", strerror(errno));
+ if (write_ref_sha1(lock, sha1, NULL) < 0)
+ die("Failed to write ref: %s.", strerror(errno));
+}
+
+int cmd_branch(int argc, const char **argv, const char *prefix)
+{
+ int delete = 0, force_delete = 0, force_create = 0;
+ int i, prefix_length;
+ const char *p;
+
+ git_config(git_default_config);
+
+ for (i = 1; i < argc; i++) {
+ const char *arg = argv[i];
+
+ if (arg[0] != '-')
+ break;
+ if (!strcmp(arg, "--")) {
+ i++;
+ break;
+ }
+ if (!strcmp(arg, "-d")) {
+ delete = 1;
+ continue;
+ }
+ if (!strcmp(arg, "-D")) {
+ delete = 1;
+ force_delete = 1;
+ continue;
+ }
+ if (!strcmp(arg, "-f")) {
+ force_create = 1;
+ continue;
+ }
+ if (!strcmp(arg, "-r")) {
+ remote_only = 1;
+ continue;
+ }
+ die(builtin_branch_usage);
+ }
+
+ prefix_length = strlen(git_path("refs/heads/"));
+ p = resolve_ref(git_path("HEAD"), head_sha1, 0);
+ if (!p)
+ die("Failed to resolve HEAD as a valid ref");
+ head = strdup(p + prefix_length);
+
+ if (delete)
+ delete_branches(argc - i, argv + i, force_delete);
+ else if (i == argc && remote_only)
+ for_each_remote_ref(show_reference);
+ else if (i == argc)
+ for_each_branch_ref(show_reference);
+ else if (argc - i == 1)
+ create_branch (argv[i], head, force_create);
+ else
+ create_branch (argv[i], argv[i + 1], force_create);
+
+ return 0;
+}
diff --git a/builtin.h b/builtin.h
index ade58c4..eb28986 100644
--- a/builtin.h
+++ b/builtin.h
@@ -15,6 +15,7 @@ extern int write_tree(unsigned char *sha
extern int cmd_add(int argc, const char **argv, const char *prefix);
extern int cmd_apply(int argc, const char **argv, const char *prefix);
+extern int cmd_branch(int argc, const char **argv, const char *prefix);
extern int cmd_cat_file(int argc, const char **argv, const char *prefix);
extern int cmd_checkout_index(int argc, const char **argv, const char *prefix);
extern int cmd_check_ref_format(int argc, const char **argv, const char *prefix);
diff --git a/git-branch.sh b/git-branch.sh
deleted file mode 100755
index e0501ec..0000000
--- a/git-branch.sh
+++ /dev/null
@@ -1,130 +0,0 @@
-#!/bin/sh
-
-USAGE='[-l] [(-d | -D) <branchname>] | [[-f] <branchname> [<start-point>]] | -r'
-LONG_USAGE='If no arguments, show available branches and mark current branch with a star.
-If one argument, create a new branch <branchname> based off of current HEAD.
-If two arguments, create a new branch <branchname> based off of <start-point>.'
-
-SUBDIRECTORY_OK='Yes'
-. git-sh-setup
-
-headref=$(git-symbolic-ref HEAD | sed -e 's|^refs/heads/||')
-
-delete_branch () {
- option="$1"
- shift
- for branch_name
- do
- case ",$headref," in
- ",$branch_name,")
- die "Cannot delete the branch you are on." ;;
- ,,)
- die "What branch are you on anyway?" ;;
- esac
- branch=$(cat "$GIT_DIR/refs/heads/$branch_name") &&
- branch=$(git-rev-parse --verify "$branch^0") ||
- die "Seriously, what branch are you talking about?"
- case "$option" in
- -D)
- ;;
- *)
- mbs=$(git-merge-base -a "$branch" HEAD | tr '\012' ' ')
- case " $mbs " in
- *' '$branch' '*)
- # the merge base of branch and HEAD contains branch --
- # which means that the HEAD contains everything in both.
- ;;
- *)
- echo >&2 "The branch '$branch_name' is not a strict subset of your current HEAD.
-If you are sure you want to delete it, run 'git branch -D $branch_name'."
- exit 1
- ;;
- esac
- ;;
- esac
- rm -f "$GIT_DIR/logs/refs/heads/$branch_name"
- rm -f "$GIT_DIR/refs/heads/$branch_name"
- echo "Deleted branch $branch_name."
- done
- exit 0
-}
-
-ls_remote_branches () {
- git-rev-parse --symbolic --all |
- sed -ne 's|^refs/\(remotes/\)|\1|p' |
- sort
-}
-
-force=
-create_log=
-while case "$#,$1" in 0,*) break ;; *,-*) ;; *) break ;; esac
-do
- case "$1" in
- -d | -D)
- delete_branch "$@"
- exit
- ;;
- -r)
- ls_remote_branches
- exit
- ;;
- -f)
- force="$1"
- ;;
- -l)
- create_log="yes"
- ;;
- --)
- shift
- break
- ;;
- -*)
- usage
- ;;
- esac
- shift
-done
-
-case "$#" in
-0)
- git-rev-parse --symbolic --branches |
- sort |
- while read ref
- do
- if test "$headref" = "$ref"
- then
- pfx='*'
- else
- pfx=' '
- fi
- echo "$pfx $ref"
- done
- exit 0 ;;
-1)
- head=HEAD ;;
-2)
- head="$2^0" ;;
-esac
-branchname="$1"
-
-rev=$(git-rev-parse --verify "$head") || exit
-
-git-check-ref-format "heads/$branchname" ||
- die "we do not like '$branchname' as a branch name."
-
-if [ -e "$GIT_DIR/refs/heads/$branchname" ]
-then
- if test '' = "$force"
- then
- die "$branchname already exists."
- elif test "$branchname" = "$headref"
- then
- die "cannot force-update the current branch."
- fi
-fi
-if test "$create_log" = 'yes'
-then
- mkdir -p $(dirname "$GIT_DIR/logs/refs/heads/$branchname")
- touch "$GIT_DIR/logs/refs/heads/$branchname"
-fi
-git update-ref -m "branch: Created from $head" "refs/heads/$branchname" $rev
diff --git a/git.c b/git.c
index 930998b..5738cb4 100644
--- a/git.c
+++ b/git.c
@@ -226,6 +226,7 @@ static void handle_internal_command(int
} commands[] = {
{ "add", cmd_add, RUN_SETUP },
{ "apply", cmd_apply },
+ { "branch", cmd_branch, RUN_SETUP },
{ "cat-file", cmd_cat_file, RUN_SETUP },
{ "checkout-index", cmd_checkout_index, RUN_SETUP },
{ "check-ref-format", cmd_check_ref_format },
^ permalink raw reply related
* Re: git refuses to switch to older branches
From: Josef Weidendorfer @ 2006-08-20 19:11 UTC (permalink / raw)
To: git
In-Reply-To: <Pine.LNX.4.63.0608201447470.28360@wbgn013.biozentrum.uni-wuerzburg.de>
On Sunday 20 August 2006 14:48, you wrote:
> Not necessarily. Sometimes you have files in your working directory, which
> are not in your repository, you know?
Yet, I think it is quite inconvenient the way it is now. The same happens if
you track a generated file by accident, and remove it later from tracking.
The real problem is that there are two reasons to put a file into .gitignore:
You do not want to have it tracked and polluting git-status because
(1) it is temporary and generated
(2) it is private and not supposed to be in the repository
To distinguish these cases, we could introduce a per-repository configuration
file .git/nevertouch . Comments?
Josef
^ permalink raw reply
* Re: Object hash (was: Re: [ANNOUNCE] git-rev-size: calculate sizes of repository)
From: Johannes Schindelin @ 2006-08-20 18:47 UTC (permalink / raw)
To: Josef Weidendorfer; +Cc: git
In-Reply-To: <200608202041.19644.Josef.Weidendorfer@gmx.de>
Hi,
On Sun, 20 Aug 2006, Josef Weidendorfer wrote:
> On Sunday 20 August 2006 18:51, Johannes Schindelin wrote:
>
> > > > +void hash_put(struct hash_map *hash, struct object *obj)
> > > > +{
> > > > + if (++hash->nr > hash->alloc / 2)
> > > > + grow_hash(hash);
> > >
> > > If you insert the same object multiple times, hash->nr will get too big.
> >
> > First, you cannot put the same object multiple times. That is not what a
> > hash does (at least in this case): it stores unique objects (identified by
> > their sha1 in this case).
>
> I put it the wrong way; I should have said "if you call hash_put() multiple
> times with the same object". You get the same index, and nothing should
> change. However, you still increment hash->nr, but this error is not really
> important as you correct it in grow_hash().
Talk about unintended side effects ;-)
Ciao,
Dscho
^ permalink raw reply
* Re: git refuses to switch to older branches
From: Johannes Schindelin @ 2006-08-20 18:46 UTC (permalink / raw)
To: Martin Waitz; +Cc: Petr Baudis, Junio C Hamano, git
In-Reply-To: <20060820181511.GH30022@admingilde.org>
Hi,
On Sun, 20 Aug 2006, Martin Waitz wrote:
> hoi :)
>
> On Sun, Aug 20, 2006 at 02:48:34PM +0200, Johannes Schindelin wrote:
> > On Sun, 20 Aug 2006, Martin Waitz wrote:
> > > On Sun, Aug 20, 2006 at 12:44:57AM +0200, Petr Baudis wrote:
> > > > Dear diary, on Sun, Aug 20, 2006 at 12:39:20AM CEST, I got a letter
> > > > where Junio C Hamano <junkio@cox.net> said that...
> > > > > But I would suggest you to be _extremely_ careful if you want to
> > > > > try this. I do not have an example offhand, but I would not be
> > > > > surprised at all if there is a valid use case where it is useful
> > > > > to have a pattern that matches a tracked file in .gitignore
> > > > > file.
> > > >
> > > > *.o and binary blobs of closed-source software.
> > >
> > > but if you want to switch from one branch which has the .o file
> > > built from source to another branch which has the .o file tracked
> > > in binary form, wouldn't you want to remove the generated file
> > > in order to store the tracked one from the new branch?
> >
> > Not necessarily. Sometimes you have files in your working directory, which
> > are not in your repository, you know?
>
> Sure. But we are only talking about files which are explicitly
> ignored in one branch and are tracked in another branch.
>
> Perhaps it makes sense to check that the file is _not_ ignored in the
> other branch (in which it is tracked).
> Would such a check make everybody happy?
Actually, I am very happy with the strict checking, and that I can
override it with "-f".
Ciao,
Dscho
^ permalink raw reply
* Re: [ANNOUNCE] git-rev-size: calculate sizes of repository
From: Johannes Schindelin @ 2006-08-20 18:44 UTC (permalink / raw)
To: Rutger Nijlunsing; +Cc: git
In-Reply-To: <20060820172458.GA21362@nospam.com>
Hi,
On Sun, 20 Aug 2006, Rutger Nijlunsing wrote:
> On Sun, Aug 20, 2006 at 06:09:34PM +0200, Johannes Schindelin wrote:
> > Hi,
> >
> > On Sun, 20 Aug 2006, Rutger Nijlunsing wrote:
> >
> > > On Sun, Aug 20, 2006 at 03:20:19PM +0200, Johannes Schindelin wrote:
> > > > Hi,
> > > >
> > > > On Sun, 20 Aug 2006, Rutger Nijlunsing wrote:
> > > >
> > > > > You can also find it on http://www.wingding.demon.nl/git-rev-size.rb
> > > >
> > > > Ruby is _so_ mainstream. Could I have a Haskell version, pretty please?
> > >
> > > I _knew_ it... Please go bug someone else. The only thing I did was
> > > help someone, and for that I choose my own tools since I do it for
> > > fun.
> >
> > Fair enough.
> >
> > -- 8< --
> > [PATCH] Add git-rev-size
> >
> > This tool spits out the number of trees, the number of blobs, and the total
> > bytes of the blobs for a given rev range.
> >
> > Most notably, it adds an object hash map structure to the library.
> >
> > Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
>
>
> [Hm, the itch seems to be contagious. Better watch out...]
>
> Small comments:
>
> The 'git-rev-size' name was chosen because originally it understood
> the same arguments as git-rev-list. You might want to add this popen()
> back, or have some other way to share those (might be simple in C). Or
> is setup_revisions() enough to have the power of git-rev-list?
It is enough. That is the beauty of setup_revisions().
> If seperate commits have to be given on the command line instead of a
> range, the command line limit is hit quite quickly (~780 commits). And
> if you'll be using xargs, the hash / cache will be less of an advantage.
Certainly. But I doubt that you'll use this command all that often.
However, it was a nice example of how easy it is to write a git builtin ;-)
> The original request was 'for each commit' to get an idea of the size
> growth during a project.
Since the arguments are the same as for git-rev-list, this is easy enough.
> 'builtin_rev_size_usage' is not referred to in the patch, only defined.
True.
-- 8< --
[PATCH] rev-size: actually show usage
Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
---
builtin-rev-size.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/builtin-rev-size.c b/builtin-rev-size.c
index ad88e48..184f926 100644
--- a/builtin-rev-size.c
+++ b/builtin-rev-size.c
@@ -78,6 +78,9 @@ int cmd_rev_size(int argc, const char **
revs.commit_format = CMIT_FMT_UNSPECIFIED;
argc = setup_revisions(argc, argv, &revs, NULL);
+ if (revs.pending.nr == 0)
+ usage(builtin_rev_size_usage);
+
prepare_revision_walk(&revs);
while ((commit = get_revision(&revs))) {
--
1.4.2.ga5e8f-dirty
^ permalink raw reply related
* Re: Object hash (was: Re: [ANNOUNCE] git-rev-size: calculate sizes of repository)
From: Josef Weidendorfer @ 2006-08-20 18:41 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.63.0608201846110.28360@wbgn013.biozentrum.uni-wuerzburg.de>
On Sunday 20 August 2006 18:51, Johannes Schindelin wrote:
> > > +static unsigned int hash_index(struct hash_map *hash, const char *sha1)
> > > +{
> > > + unsigned int index = *(unsigned int *)sha1;
> >
> > If you have the same SHA1, stored at different addresses, you get different
> > indexes for the same SHA1. Index probably should be calculated from the
> > SHA1 string.
>
> Actually, it does! "*(unsigned int *)sha1" means that the first 4 bytes
> of the sha1 are interpreted as a number.
Ah, yes. That's fine.
> > > +void hash_put(struct hash_map *hash, struct object *obj)
> > > +{
> > > + if (++hash->nr > hash->alloc / 2)
> > > + grow_hash(hash);
> >
> > If you insert the same object multiple times, hash->nr will get too big.
>
> First, you cannot put the same object multiple times. That is not what a
> hash does (at least in this case): it stores unique objects (identified by
> their sha1 in this case).
I put it the wrong way; I should have said "if you call hash_put() multiple
times with the same object". You get the same index, and nothing should
change. However, you still increment hash->nr, but this error is not really
important as you correct it in grow_hash().
So... sorry for the noise ;-)
Josef
^ permalink raw reply
* Re: [PATCH] Helper function to shell quote all arg values at once.
From: Christian Couder @ 2006-08-20 18:32 UTC (permalink / raw)
To: Junio Hamano; +Cc: git
In-Reply-To: <20060820080749.e31276dd.chriscool@tuxfamily.org>
I forgot to sign this patch off :
> The new sq_quote_argv function is used to refactor the
> tracing code in "git.c" and "exec_cmd.c".
> This function allocates memory and fills it with a string
> containing the quoted argument values. Then it returns a
> pointer to this memory that must be freed afterwards.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Christian.
^ permalink raw reply
* Re: git refuses to switch to older branches
From: Martin Waitz @ 2006-08-20 18:15 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Petr Baudis, Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.63.0608201447470.28360@wbgn013.biozentrum.uni-wuerzburg.de>
[-- Attachment #1: Type: text/plain, Size: 1481 bytes --]
hoi :)
On Sun, Aug 20, 2006 at 02:48:34PM +0200, Johannes Schindelin wrote:
> On Sun, 20 Aug 2006, Martin Waitz wrote:
> > On Sun, Aug 20, 2006 at 12:44:57AM +0200, Petr Baudis wrote:
> > > Dear diary, on Sun, Aug 20, 2006 at 12:39:20AM CEST, I got a letter
> > > where Junio C Hamano <junkio@cox.net> said that...
> > > > But I would suggest you to be _extremely_ careful if you want to
> > > > try this. I do not have an example offhand, but I would not be
> > > > surprised at all if there is a valid use case where it is useful
> > > > to have a pattern that matches a tracked file in .gitignore
> > > > file.
> > >
> > > *.o and binary blobs of closed-source software.
> >
> > but if you want to switch from one branch which has the .o file
> > built from source to another branch which has the .o file tracked
> > in binary form, wouldn't you want to remove the generated file
> > in order to store the tracked one from the new branch?
>
> Not necessarily. Sometimes you have files in your working directory, which
> are not in your repository, you know?
Sure. But we are only talking about files which are explicitly
ignored in one branch and are tracked in another branch.
Perhaps it makes sense to check that the file is _not_ ignored in the
other branch (in which it is tracked).
Would such a check make everybody happy?
Is there an easy way to check if some file is marked as ignored in
an other branch?
--
Martin Waitz
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: Object hash (was: Re: [ANNOUNCE] git-rev-size: calculate sizes of repository)
From: Rutger Nijlunsing @ 2006-08-20 17:40 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Josef Weidendorfer, Rutger Nijlunsing, git
In-Reply-To: <Pine.LNX.4.63.0608201846110.28360@wbgn013.biozentrum.uni-wuerzburg.de>
> Second, since you call hash_put() once per object, hash->nr cannot grow
> too big, because grow_hash() doubles hash->alloc. And I call grow_hash()
> once the hash map is half-full; Somebody once told me that would be the
> optimal growing strategy.
Optimal growing mainly means to be O(n) (amortized) after n
inserts. That translates to at least _doubling_ (factor 2 or more) the
capacity once you're too full.
Assume doubling at a percentage full. Assume realloc(s) takes O(s)
(where s = number of bytes). Assume we start with 1 element.
We realloc() then when we've got 1 element, then at 2, 4, 8 etc. The
size of the realloc() at each point will also be 1, 2, 4, 8
etc. However, this cost of O(s) can be amortized over the number of
elements. So the work done _per insert_ is still a constant (amortized
again).
Ascilly:
x x x x x x x x x x ... (each insert)
R R R ... (each realloc)
1 2 0 4 0 0 0 8 0 0 ... (cost of those realloc())
This has also to do with the infinite series of the sum(k>0) of 2^-k
being a constant.
--
Rutger Nijlunsing ---------------------------------- eludias ed dse.nl
never attribute to a conspiracy which can be explained by incompetence
----------------------------------------------------------------------
^ 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;
as well as URLs for NNTP newsgroup(s).