* Re: merge summaries
From: Junio C Hamano @ 2007-05-20 9:07 UTC (permalink / raw)
To: Steffen Prohaska; +Cc: Linus Torvalds, Git Mailing List
In-Reply-To: <7vwsz3rjb6.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano <junkio@cox.net> writes:
> Steffen Prohaska <prohaska@zib.de> writes:
>
>> But this is true not only for the upstream master but for every branch.
>> The first-parent-path of any commit documents (at least in summaries)
>> which features were introduced to the branch at which place. If a
>> topic pulls from upstream, features already available in the upstream
>> are introduced to the topic at this time. Why not summarize this?
Because it does not matter from the overall picture at that
point. In an earlier message I said that the summary is useful
in a very short term -- the summary messages are useful while
the person who did the merge (or somebody who is simply
following him) is looking at near the tip of the branch, but
when you are later viewing the older parts of the history from
10,000 feet above, the distinction between what were pulled into
the local branch of the person who made the merge vs what the
person already had on the branch when he made the merge becomes
more or less irrelevant. The end result is that the project now
got the fruits of labor that happened on both sides.
> Strictly speaking, it is not even correct with the upstream
> master. I believe even my own 'maint' has fast forward merge
> that I pulled from other people, and that part of the history if
> you follow first-parent, you would not be following the "project's"
> history (if there is such a thing, and if you are equating it
> with "JC's view of the history").
Regrettably, I have to say that --first-parent is, although it is
often a useful way to get an approximation, a bad option that
tends to instill a wrong mental model to people new to git.
In git, unlike CVS and SVN (I do not know now Monotone,
Mercurial and BitKeeper's branches work with this respect),
branches in git do not have their "own" identity in global
history that is recorded in the commit ancestry graph. The only
way you can ask "how did this branch come about to this state?"
is to look at your own reflog, and that is strictly a local
thing (i.e. the history as you observed it).
Even when you are "the upstream maintainer" [*1*], there are
cases where you pushed your tips out to the open, other people
built their work on top of it while you were not adding anything
to your tree, and you end up pulling from them. Such a pull
will result in a fast-forward merge, and that is rightly so.
They did all the work, and the collective history of the project
during that time literally followed what the other person did,
not the "upstream maintainer". However, from the point of view
of somebody who is following the upstream maintainer's tree,
what he would observe is that at one point of the time the tip
of the tree was the last commit the upstream maintainer pushed
out (which became the base of the other person's work), and then
the next time you look at the tree there are bunch of commits by
the other person, appearing all at once even though there is no
"real" merge commit that summarizes the work done on the "side
branch". Which some of the readers might feel a bit confusing
and inconsistent, but that is really the correct interpretation
of the history. The reason I feel --first-parent helps this
confusion is that people would think it would be consistent if
we do not allow fast-forward merges and instead always forced a
real merge in such a case (then you would consistently get the
merge summary). In other words, the first parent is not
inherently special -- and people should view history that way,
but --first-parent and merge.summary give this false impression
that it is somehow special.
Anyhow, enough rambling for a day. Now I have two releases
to do tonight ;-)
[Footnote]
*1* In git, strictly speaking, there is no up or down, but as a
social convention, I get updates from Shawn, Paulus, Bruce and
few other people via pull, and it would not be too incorrect to
say I'm more upstream than they are, and all others are more
downstream.
^ permalink raw reply
* Re: merge summaries
From: Junio C Hamano @ 2007-05-20 8:27 UTC (permalink / raw)
To: Steffen Prohaska; +Cc: Linus Torvalds, Git Mailing List
In-Reply-To: <0E590255-A3F4-4D36-B798-80364ACD0D06@zib.de>
Steffen Prohaska <prohaska@zib.de> writes:
> But this is true not only for the upstream master but for every branch.
> The first-parent-path of any commit documents (at least in summaries)
> which features were introduced to the branch at which place. If a
> topic pulls from upstream, features already available in the upstream
> are introduced to the topic at this time. Why not summarize this?
Strictly speaking, it is not even correct with the upstream
master. I believe even my own 'maint' has fast forward merge
that I pulled from other people, and that part of the history if
you follow first-parent, you would not be following the "project's"
history (if there is such a thing, and if you are equating it
with "JC's view of the history").
^ permalink raw reply
* Re: merge summaries
From: Steffen Prohaska @ 2007-05-20 8:17 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Linus Torvalds, Git Mailing List
In-Reply-To: <7vy7jkva41.fsf@assigned-by-dhcp.cox.net>
On May 19, 2007, at 10:20 PM, Junio C Hamano wrote:
> Junio C Hamano <junkio@cox.net> writes:
>
>> As long as you stay as a "pull-only" person, and will never ask
>> the upstream to pull back from you, what first parent log would
>> show would make perfectly good sense. However, once a pull from
>> you by the upstream happens, the resulting history would get the
>> summary in the merge you did, which is totally unwanted --- the
>> upstream already had those summarized commits long time ago in
>> its history.
Why would the summary of the upstream-to-topic-merge be so harmful?
It would not change the summary message of a pull from topic back to
the upstream. Af far as I can see, merge commits do not contribute
to a summary message. However, when browsing the full log the summary
might be distracting because a single commit may be listed at
various places, although it was already in the upstream. This is
kind of ugly. ...
> The criteria to decide "wanted vs unwanted" all boil down to
> essentially the question of who "owns" the history. Although
> Linus often says that the point of distributed development is
> that nobody is special, at the end of day, once you asked the
> "upstream" to pull from you, your history becomes part of HIS
> history, and at that point, when HE browses the history (which
> he now thinks as HIS history), the summary shows the summary the
> wrong way. So in that sense, merge.summary is going against the
> philosophy of distributedness.
... I'd say you only "own" the history of a branch along its
first-parent-path to the initial commit. This is what you control
either by directly committing to the branch or by pulling other
branches.
Isn't the history along the first parents especially important because
it documents the functionality of the project? If I want to say
something
about git at a certain point in time, I'd talk about master of the
official
git repository. "git log --first-parent" in some sense completely
describes the history of git. At merges at least a summary of features
pulled from topic branches is given.
But this is true not only for the upstream master but for every branch.
The first-parent-path of any commit documents (at least in summaries)
which features were introduced to the branch at which place. If a
topic pulls from upstream, features already available in the upstream
are introduced to the topic at this time. Why not summarize this?
> The summary message is useful in a very short term (i.e. while
> it is still near the tip of the branch and running "git log"),
> but I think JBF is correct to point out that it is showing a
> shortcoming of our history browsing tools. The information
> recorded there can easily be reproduced on the fly. Whenever
> you see a merge commit, you can choose one parent to base your
> view of the history upon (typically the first parent), and
> summarize commits that are reachable from other parents but not
> from that parent (e.g. for a typical two-parent merge, you would
> run "git short-log $commit^1..$commit^2").
I find the summary messages quite useful when browsing merges. The
summary
allows to quickly locate the branch that was merged at that point.
You just
look at the list of summaries and locate the mentioned commits in gitk.
Minimal merge messages like "Merge branch 'maint' of URL into maint" are
much harder to understand. The branches mentioned in the message do
no longer
have any meaning. They either ceased to exist or advanced to
somewhere else.
The URL typically means nothing to me, although it may have meant a
lot to
the one who pulled at the time of the pull. Such minimal messages do
not help browsing history.
It would probably be more useful to name the merged branch by the
summary
of its head at the time of the merge. For example
"Merge branch 'user-manual: reorganize public git repo discussion'"
would give more hints where to continue browsing than
"Merge branch 'maint' of URL into maint".
- Steffen
^ permalink raw reply
* Re: [PATCH] Tech 'git-apply' that spaces before a tab are candidate for stripping
From: Junio C Hamano @ 2007-05-20 7:53 UTC (permalink / raw)
To: Marco Costalba; +Cc: Git Mailing List
In-Reply-To: <e5bfff550705190944y44cb57baiccb139b1d649face@mail.gmail.com>
"Marco Costalba" <mcostalba@gmail.com> writes:
> Currently 'git-apply' identify as stealth space a line starting with
> spaces followed by a tab.
>
> This patch updates the algorithm to find spaces + tab also in the
> middle of a line and not only at the beginning.
>
> As example lines like
>
> "#define MY_VALUE \t 1"
>
> are now identified and cleaned up if option '--whitespace=strip' is
> used.
I specifically did not want to do the above, as it would also
munge something like this:
const char string[] = "abc <HT> de";
^ permalink raw reply
* Re: Commit ID in exported Tar Ball
From: Shawn O. Pearce @ 2007-05-20 3:57 UTC (permalink / raw)
To: Junio C Hamano
Cc: René Scharfe, Frank Lichtenheld, Johan Herland, git,
Thomas Glanzmann, Michael Gernoth
In-Reply-To: <7vd50wv88t.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano <junkio@cox.net> wrote:
> René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
>
> > Something like the following patch? Since we're already embedding the
> > commit ID in a comment, we might as well offer creating a synthetic file
> > for it, too, if that solves a user's problem that might be difficult to
> > work around otherwise.
What about being able to get the output of git-describe embedded
into an archive file? Doesn't git.git do that in its Makefile? ;-)
git-describe is more human-friendly than a SHA-1...
--
Shawn.
^ permalink raw reply
* Re: [PATCH v2] Add an option to git-ls-tree to display also the size of blob
From: Shawn O. Pearce @ 2007-05-20 3:54 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Junio C Hamano, git
In-Reply-To: <200705192208.11905.jnareb@gmail.com>
Jakub Narebski <jnareb@gmail.com> wrote:
> Rationale: for non-blob entries size of an object has no much
> meaning, and is not very interesting. Moreover, in planned
> pack v4 tree objects would be constructed on demand, so tree
> size would need to be calculated... although isn't object size
> stored in the header?
Yes and no. In pack v4 the object sizes stored within the packfile
are more about what we need to know in order to efficiently unpack
the object than about supplying the canonical format length. If a
value is redundant, we don't store it. The canonical format length
is almost always redundant.
For blobs we still need the raw data length to unpack efficiently,
and hence we have the blob's canonical size readily available.
For trees its actually more the tree entry record count, as that
is all we need to know in order to recover the tree. For commits
we don't even need a length, but we instead have the number of
parent commits.
Since pack v4 is really about faster runtime decoding our ability
to reproduce canonical encoding of objects is reduced slightly.
I think that's OK as we actually only need the canonical encoding
infrequently (index-pack during network transfer, cat-file) and
we're not really considering pack v4 for network transfer. Yet.
> I hope this addresses concerns mentioned in this thread: the
> alternate name --size for -l/--long option, and showing size
> for tree (and commit/submodule) objects.
Yes, thanks!
--
Shawn.
^ permalink raw reply
* [PATCH 3/3] Use stringbuf to clean up some string handling code.
From: Timo Sirainen @ 2007-05-20 2:25 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: text/plain, Size: 4198 bytes --]
---
commit.c | 30 +++++++++++++-----------------
local-fetch.c | 34 ++++++++++++++++------------------
2 files changed, 29 insertions(+), 35 deletions(-)
diff --git a/commit.c b/commit.c
index bee066f..58f1718 100644
--- a/commit.c
+++ b/commit.c
@@ -6,6 +6,7 @@
#include "interpolate.h"
#include "diff.h"
#include "revision.h"
+#include "str.h"
int save_commit_buffer = 1;
@@ -821,7 +822,7 @@ static long format_commit_message(const struct
commit *commit,
ILEFT_RIGHT,
};
struct commit_list *p;
- char parents[1024];
+ stringbuf(parents, 1024);
int i;
enum { HEADER, SUBJECT, BODY } state;
@@ -853,22 +854,17 @@ static long format_commit_message(const struct
commit *commit,
? "<"
: ">");
- parents[1] = 0;
- for (i = 0, p = commit->parents;
- p && i < sizeof(parents) - 1;
- p = p->next)
- i += snprintf(parents + i, sizeof(parents) - i - 1, " %s",
- sha1_to_hex(p->item->object.sha1));
- interp_set_entry(table, IPARENTS, parents + 1);
-
- parents[1] = 0;
- for (i = 0, p = commit->parents;
- p && i < sizeof(parents) - 1;
- p = p->next)
- i += snprintf(parents + i, sizeof(parents) - i - 1, " %s",
- find_unique_abbrev(p->item->object.sha1,
- DEFAULT_ABBREV));
- interp_set_entry(table, IPARENTS_ABBREV, parents + 1);
+ str_c(parents)[1] = 0;
+ for (p = commit->parents; p; p = p->next)
+ str_printfa(parents, " %s", sha1_to_hex(p->item->object.sha1));
+ interp_set_entry(table, IPARENTS, str_c(parents) + 1);
+
+ str_c(parents)[1] = 0;
+ for (p = commit->parents; p; p = p->next)
+ str_printfa(parents, " %s",
+ find_unique_abbrev(p->item->object.sha1,
+ DEFAULT_ABBREV));
+ interp_set_entry(table, IPARENTS_ABBREV, str_c(parents) + 1);
for (i = 0, state = HEADER; msg[i] && state < BODY; i++) {
int eol;
diff --git a/local-fetch.c b/local-fetch.c
index 4b650ef..6d0599f 100644
--- a/local-fetch.c
+++ b/local-fetch.c
@@ -4,6 +4,7 @@
#include "cache.h"
#include "commit.h"
#include "fetch.h"
+#include "str.h"
static int use_link;
static int use_symlink;
@@ -21,12 +22,11 @@ static struct packed_git *packs;
static void setup_index(unsigned char *sha1)
{
struct packed_git *new_pack;
- char filename[PATH_MAX];
- strcpy(filename, path);
- strcat(filename, "/objects/pack/pack-");
- strcat(filename, sha1_to_hex(sha1));
- strcat(filename, ".idx");
- new_pack = parse_pack_index_file(sha1, filename);
+ stringbuf(filename, PATH_MAX);
+
+ str_printfa(filename, "%s/objects/pack/pack-%s.idx",
+ path, sha1_to_hex(sha1));
+ new_pack = parse_pack_index_file(sha1, str_c(filename));
new_pack->next = packs;
packs = new_pack;
}
@@ -35,10 +35,11 @@ static int setup_indices(void)
{
DIR *dir;
struct dirent *de;
- char filename[PATH_MAX];
+ stringbuf(filename, PATH_MAX);
unsigned char sha1[20];
- sprintf(filename, "%s/objects/pack/", path);
- dir = opendir(filename);
+
+ str_printfa(filename, "%s/objects/pack/", path);
+ dir = opendir(str_c(filename));
if (!dir)
return -1;
while ((de = readdir(dir)) != NULL) {
@@ -137,20 +138,17 @@ static int fetch_pack(const unsigned char *sha1)
static int fetch_file(const unsigned char *sha1)
{
static int object_name_start = -1;
- static char filename[PATH_MAX];
+ static stringbuf(filename, PATH_MAX);
char *hex = sha1_to_hex(sha1);
char *dest_filename = sha1_file_name(sha1);
if (object_name_start < 0) {
- strcpy(filename, path); /* e.g. git.git */
- strcat(filename, "/objects/");
- object_name_start = strlen(filename);
+ str_printfa(filename, "%s/objects/", path); /* e.g. git.git */
+ object_name_start = str_len(filename);
}
- filename[object_name_start+0] = hex[0];
- filename[object_name_start+1] = hex[1];
- filename[object_name_start+2] = '/';
- strcpy(filename + object_name_start + 3, hex + 2);
- return copy_file(filename, dest_filename, hex, 0);
+ str_truncate(filename, object_name_start);
+ str_printfa(filename, "%c%c/%s", hex[0], hex[1], hex + 2);
+ return copy_file(str_c(filename), dest_filename, hex, 0);
}
int fetch(unsigned char *sha1)
--
1.5.1.4
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply related
* [PATCH 2/3] Use stringbuf to fix buffer overflows due to broken use of snprintf()
From: Timo Sirainen @ 2007-05-20 2:24 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: text/plain, Size: 3252 bytes --]
---
diff.c | 51 ++++++++++++++++++++++-----------------------------
1 files changed, 22 insertions(+), 29 deletions(-)
diff --git a/diff.c b/diff.c
index 33297aa..4d8f4bc 100644
--- a/diff.c
+++ b/diff.c
@@ -9,6 +9,7 @@
#include "xdiff-interface.h"
#include "color.h"
#include "attr.h"
+#include "str.h"
#ifdef NO_FAST_WORKING_DIRECTORY
#define FAST_WORKING_DIRECTORY 0
@@ -1823,14 +1824,14 @@ static void diff_fill_sha1_info(struct diff_filespec *one)
static void run_diff(struct diff_filepair *p, struct diff_options *o)
{
const char *pgm = external_diff();
- char msg[PATH_MAX*2+300], *xfrm_msg;
+ stringbuf(msg, PATH_MAX*2+300);
+ char *xfrm_msg;
struct diff_filespec *one;
struct diff_filespec *two;
const char *name;
const char *other;
char *name_munged, *other_munged;
int complete_rewrite = 0;
- int len;
if (DIFF_PAIR_UNMERGED(p)) {
/* unmerged */
@@ -1847,30 +1848,26 @@ static void run_diff(struct diff_filepair *p, struct diff_options *o)
diff_fill_sha1_info(one);
diff_fill_sha1_info(two);
- len = 0;
switch (p->status) {
case DIFF_STATUS_COPIED:
- len += snprintf(msg + len, sizeof(msg) - len,
- "similarity index %d%%\n"
- "copy from %s\n"
- "copy to %s\n",
- (int)(0.5 + p->score * 100.0/MAX_SCORE),
- name_munged, other_munged);
+ str_printfa(msg, "similarity index %d%%\n"
+ "copy from %s\n"
+ "copy to %s\n",
+ (int)(0.5 + p->score * 100.0/MAX_SCORE),
+ name_munged, other_munged);
break;
case DIFF_STATUS_RENAMED:
- len += snprintf(msg + len, sizeof(msg) - len,
- "similarity index %d%%\n"
- "rename from %s\n"
- "rename to %s\n",
- (int)(0.5 + p->score * 100.0/MAX_SCORE),
- name_munged, other_munged);
+ str_printfa(msg, "similarity index %d%%\n"
+ "rename from %s\n"
+ "rename to %s\n",
+ (int)(0.5 + p->score * 100.0/MAX_SCORE),
+ name_munged, other_munged);
break;
case DIFF_STATUS_MODIFIED:
if (p->score) {
- len += snprintf(msg + len, sizeof(msg) - len,
- "dissimilarity index %d%%\n",
- (int)(0.5 + p->score *
- 100.0/MAX_SCORE));
+ str_printfa(msg, "dissimilarity index %d%%\n",
+ (int)(0.5 + p->score *
+ 100.0/MAX_SCORE));
complete_rewrite = 1;
break;
}
@@ -1889,19 +1886,15 @@ static void run_diff(struct diff_filepair *p, struct diff_options *o)
(!fill_mmfile(&mf, two) && file_is_binary(two)))
abbrev = 40;
}
- len += snprintf(msg + len, sizeof(msg) - len,
- "index %.*s..%.*s",
- abbrev, sha1_to_hex(one->sha1),
- abbrev, sha1_to_hex(two->sha1));
+ str_printfa(msg, "index %.*s..%.*s",
+ abbrev, sha1_to_hex(one->sha1),
+ abbrev, sha1_to_hex(two->sha1));
if (one->mode == two->mode)
- len += snprintf(msg + len, sizeof(msg) - len,
- " %06o", one->mode);
- len += snprintf(msg + len, sizeof(msg) - len, "\n");
+ str_printfa(msg, " %06o", one->mode);
+ str_append(msg, "\n");
}
- if (len)
- msg[--len] = 0;
- xfrm_msg = len ? msg : NULL;
+ xfrm_msg = str_len(msg) ? str_c(msg) : NULL;
if (!pgm &&
DIFF_FILE_VALID(one) && DIFF_FILE_VALID(two) &&
--
1.5.1.4
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply related
* [PATCH 1/3] Added generic string handling code.
From: Timo Sirainen @ 2007-05-20 2:24 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: text/plain, Size: 4647 bytes --]
Aren't you already tired of using the crappy string handling functions
that libc provides? I see a lot of really ugly code in git that exists
just because this.
I also see a lot of potential buffer overflows because either no
overflow checking is done, or it's done wrong. Perhaps it doesn't matter
now if you're manually inspecting each patch before feeding to git, but
I fear that in future someone's automated git handler will be
responsible for getting malicious code added into Linux, just because of
a simple buffer overflow that could have been easily avoided.
So here's my try on starting with something simple. Unlike almost all
other string handling libraries, it doesn't allocate the memory
dynamically. This makes it really easy to convert existing code to use
it. I'm including some example changes in the other patches. Besides
making the code safer, it can also make it faster, especially those
strcat() replacements.
I'm aware of strbuf.[ch], but I wasn't sure if I should have merged this
code with it or what. It had this "eof" field which I think makes it
more like a "file reader string" and not a "string buffer". So I just
added new str.[ch] files.
---
Makefile | 4 ++--
str.c | 40 ++++++++++++++++++++++++++++++++++++++++
str.h | 32 ++++++++++++++++++++++++++++++++
3 files changed, 74 insertions(+), 2 deletions(-)
create mode 100644 str.c
create mode 100644 str.h
diff --git a/Makefile b/Makefile
index 29243c6..f61ad50 100644
--- a/Makefile
+++ b/Makefile
@@ -294,7 +294,7 @@ XDIFF_LIB=xdiff/lib.a
LIB_H = \
archive.h blob.h cache.h commit.h csum-file.h delta.h grep.h \
diff.h object.h pack.h pkt-line.h quote.h refs.h list-objects.h sideband.h \
- run-command.h strbuf.h tag.h tree.h git-compat-util.h revision.h \
+ run-command.h strbuf.h str.h tag.h tree.h git-compat-util.h revision.h \
tree-walk.h log-tree.h dir.h path-list.h unpack-trees.h builtin.h \
utf8.h reflog-walk.h patch-ids.h attr.h decorate.h progress.h mailmap.h
@@ -312,7 +312,7 @@ LIB_OBJS = \
object.o pack-check.o pack-write.o patch-delta.o path.o pkt-line.o \
sideband.o reachable.o reflog-walk.o \
quote.o read-cache.o refs.o run-command.o dir.o object-refs.o \
- server-info.o setup.o sha1_file.o sha1_name.o strbuf.o \
+ server-info.o setup.o sha1_file.o sha1_name.o strbuf.o str.o \
tag.o tree.o usage.o config.o environment.o ctype.o copy.o \
revision.o pager.o tree-walk.o xdiff-interface.o \
write_or_die.o trace.o list-objects.o grep.o match-trees.o \
diff --git a/str.c b/str.c
new file mode 100644
index 0000000..d46e7f4
--- /dev/null
+++ b/str.c
@@ -0,0 +1,40 @@
+#include "str.h"
+
+void _str_append(struct string *str, const char *cstr)
+{
+ unsigned int avail = str->size - str->len;
+ unsigned int len = strlen(cstr);
+
+ if (len >= avail) {
+ len = avail - 1;
+ str->overflowed = 1;
+ }
+ memcpy(str->buf + str->len, cstr, len);
+ str->len += len;
+ str->buf[str->len] = '\0';
+}
+
+void _str_printfa(struct string *str, const char *fmt, ...)
+{
+ unsigned int avail = str->size - str->len;
+ va_list va;
+ int ret;
+
+ va_start(va, fmt);
+ ret = vsnprintf(str->buf + str->len, avail, fmt, va);
+ if (ret < avail)
+ str->len += ret;
+ else {
+ str->len += avail - 1;
+ str->overflowed = 1;
+ }
+ va_end(va);
+}
+
+void _str_truncate(struct string *str, unsigned int len)
+{
+ if (len >= str->size)
+ len = str->size - 1;
+ str->len = len;
+ str->buf[len] = '\0';
+}
diff --git a/str.h b/str.h
new file mode 100644
index 0000000..99d3215
--- /dev/null
+++ b/str.h
@@ -0,0 +1,32 @@
+#ifndef STR_H
+#define STR_H
+
+#include "git-compat-util.h"
+
+struct string {
+ unsigned int size;
+ unsigned int len:31;
+ unsigned int overflowed:1;
+ char buf[];
+};
+
+#define stringbuf(name, size) \
+ union { \
+ struct string string; \
+ char string_buf[sizeof(struct string) + (size) + 1]; \
+ } name = { { (size)+1, 0, 0 } }
+
+extern void _str_append(struct string *str, const char *cstr);
+extern void _str_printfa(struct string *str, const char *fmt, ...)
+ __attribute__((format (printf, 2, 3)));
+extern void _str_truncate(struct string *str, unsigned int len);
+
+#define str_append(str, cstr) _str_append(&(str).string, cstr)
+#define str_printfa(str, fmt, ...) _str_printfa(&(str).string, fmt, __VA_ARGS__)
+#define str_truncate(str, len) _str_truncate(&(str).string, len)
+
+#define str_c(str) ((str).string.buf)
+#define str_len(str) ((str).string.len)
+#define str_overflowed(str) ((str).string.overflowed)
+
+#endif
--
1.5.1.4
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply related
* Re: [PATCH] cvsserver: Don't send mixed messages to clients
From: Frank Lichtenheld @ 2007-05-20 1:10 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Martin Langhoff, git
In-Reply-To: <7v646otizs.fsf@assigned-by-dhcp.cox.net>
On Sat, May 19, 2007 at 05:51:35PM -0700, Junio C Hamano wrote:
> Frank Lichtenheld <frank@lichtenheld.de> writes:
>
> > On Mon, May 14, 2007 at 02:28:29PM -0700, Junio C Hamano wrote:
> >> Frank Lichtenheld <frank@lichtenheld.de> writes:
> >> > For the record, cvs (server side) does the following in this particular situation:
> >> > buf_output0 (buf_to_net, "I HATE YOU\n");
> >> > buf_flush (buf_to_net, true);
> >> >
> >> > /* Don't worry about server_cleanup, server_active isn't set
> >> > yet. */
> >> > exit (EXIT_FAILURE);
> >>
> >> Ok, I stand corrected. If there is a pattern to emulate, that
> >> should be found in the real cvs server.
> >
> > Will you correct the patch then back to my version? master still has
> > your version of it AFAICT.
>
> Will do, but then we would also have to vet all the error exit
> paths to match what the real cvsserver does, wouldn't we?
Sure. But it is nothing utterly important. It just so happens
that I already have a test for this one case and would like to
see it fixed before submitting the test patches again.
> We've survived without that so far, so while I'll apply the
> above change before 1.5.2, I think we do not have to have the
> release wait for fixes to the other error exit paths.
Agreed.
Gruesse,
--
Frank Lichtenheld <frank@lichtenheld.de>
www: http://www.djpig.de/
^ permalink raw reply
* Re: [PATCH] cvsserver: Don't send mixed messages to clients
From: Junio C Hamano @ 2007-05-20 0:51 UTC (permalink / raw)
To: Frank Lichtenheld; +Cc: Martin Langhoff, git
In-Reply-To: <20070520004543.GF4085@planck.djpig.de>
Frank Lichtenheld <frank@lichtenheld.de> writes:
> On Mon, May 14, 2007 at 02:28:29PM -0700, Junio C Hamano wrote:
>> Frank Lichtenheld <frank@lichtenheld.de> writes:
>> > For the record, cvs (server side) does the following in this particular situation:
>> > buf_output0 (buf_to_net, "I HATE YOU\n");
>> > buf_flush (buf_to_net, true);
>> >
>> > /* Don't worry about server_cleanup, server_active isn't set
>> > yet. */
>> > exit (EXIT_FAILURE);
>>
>> Ok, I stand corrected. If there is a pattern to emulate, that
>> should be found in the real cvs server.
>
> Will you correct the patch then back to my version? master still has
> your version of it AFAICT.
Will do, but then we would also have to vet all the error exit
paths to match what the real cvsserver does, wouldn't we?
We've survived without that so far, so while I'll apply the
above change before 1.5.2, I think we do not have to have the
release wait for fixes to the other error exit paths.
^ permalink raw reply
* Re: [PATCH] cvsserver: Don't send mixed messages to clients
From: Frank Lichtenheld @ 2007-05-20 0:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Martin Langhoff, git
In-Reply-To: <7vmz07m6wy.fsf@assigned-by-dhcp.cox.net>
On Mon, May 14, 2007 at 02:28:29PM -0700, Junio C Hamano wrote:
> Frank Lichtenheld <frank@lichtenheld.de> writes:
> > For the record, cvs (server side) does the following in this particular situation:
> > buf_output0 (buf_to_net, "I HATE YOU\n");
> > buf_flush (buf_to_net, true);
> >
> > /* Don't worry about server_cleanup, server_active isn't set
> > yet. */
> > exit (EXIT_FAILURE);
>
> Ok, I stand corrected. If there is a pattern to emulate, that
> should be found in the real cvs server.
Will you correct the patch then back to my version? master still has
your version of it AFAICT.
Gruesse,
--
Frank Lichtenheld <frank@lichtenheld.de>
www: http://www.djpig.de/
^ permalink raw reply
* Re: [3/4] What's not in 1.5.2 (new topics)
From: Petr Baudis @ 2007-05-20 0:16 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Julian Phillips, Junio C Hamano, git
In-Reply-To: <7vsl9u55tv.fsf@assigned-by-dhcp.cox.net>
On Fri, May 18, 2007 at 08:45:16PM CEST, Junio C Hamano wrote:
> Julian Phillips <julian@quantumfyre.co.uk> writes:
>
> > On Fri, 18 May 2007, Junio C Hamano wrote:
> >
> >> Jakub Narebski <jnareb@gmail.com> writes:
> >>
> >>> [Cc: Petr Baudis <pasky@suse.cz>, Josef Weidendorfer
> >>> <Josef.Weidendorfer@gmx.de>, "Michael S. Tsirkin" <mst@dev.mellanox.co.il>,
> >>> Junio C Hamano <junkio@cox.net>, Andy Parkins <andyparkins@gmail.com>,
> >>> Nicolas Pitre <nico@cam.org>, git@vger.kernel.org]
> >>
> >> Offtopic. Why do you do this, and what benefit are you or
> >> anybody in the above list, which is in body part of the message,
> >> getting?
> >
> > It looks like he is posting through gmane using a news reader ... so
> > the list post comes from gmane while the CCs go out directly (I
> > assume).
>
> Ah, I see. The names listed on that in-body CC: do appear on
> the To: in the copy of the message that came via e-mail. If
> that is how gmane operates then there is nothing Jakub to do to
> improve it, I guess...
>
> Thanks for the clarification.
Actually, Jakub, if it can be turned off, could you, please?
Not getting cc'd on replies is slightly annoying. But this is highly
confusing - suddenly I must take care _not_ to reply to the private
copies, and we actually do have a parallel subthread of replies to your
mail not cc'd to the mailing list. :-(
--
Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
Ever try. Ever fail. No matter. // Try again. Fail again. Fail better.
-- Samuel Beckett
^ permalink raw reply
* Re: Commit ID in exported Tar Ball
From: René Scharfe @ 2007-05-20 0:15 UTC (permalink / raw)
To: Junio C Hamano
Cc: Frank Lichtenheld, Johan Herland, git, Thomas Glanzmann,
Michael Gernoth
In-Reply-To: <7vd50wv88t.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano schrieb:
> René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
>
>> Something like the following patch? Since we're already embedding
>> the commit ID in a comment, we might as well offer creating a
>> synthetic file for it, too, if that solves a user's problem that
>> might be difficult to work around otherwise.
>
> Are you detecting and erroring out if the named file already exists
> in the tree being archived?
>
> Should we? Maybe we should just replace with warning?
Currently the commit ID file is appended to the archive, so if there is
a name conflict, we keep both files. unzip offers a choice between
renaming, overwriting and keeping the first extracted version when the
second one is about to be extracted. tar has a -k option: with -k you
get the first version, without -k you get the second one.
To avoid confusion, we should disallow a name that already comes from
the tree. Only I can't see an easy way to implement this. Perhaps a
check using get_tree_entry is enough -- it doesn't take pathspecs into
account, though. That means we would disallow all names in the tree,
even if a pathspec excludes the chosen commit ID file name.
Before I roll my own path existence checker with pathspec support, is
there something like that already implemented? I suspect it's hiding in
the diff code, but I don't dare go near it. ;-)
> Also should we silently ignore the request if the tree-ish is not a
> commit-ish, or error out?
As Angry said, erroring out is better. It's also easy to do.
Documentation/git-archive.txt | 4 ++++
archive-tar.c | 7 +++++++
archive-zip.c | 7 +++++++
archive.h | 1 +
builtin-archive.c | 32 ++++++++++++++++++++++++++++++++
5 files changed, 51 insertions(+), 0 deletions(-)
diff --git a/Documentation/git-archive.txt b/Documentation/git-archive.txt
index 721e035..7016d1e 100644
--- a/Documentation/git-archive.txt
+++ b/Documentation/git-archive.txt
@@ -43,6 +43,10 @@ OPTIONS
--prefix=<prefix>/::
Prepend <prefix>/ to each filename in the archive.
+--commit-id-file=<filename>::
+ Adds a file to the archive containing the commit ID. This option
+ is can only be used if <tree-ish> references a commit or tag.
+
<extra>::
This can be any options that the archiver backend understand.
See next section.
diff --git a/archive-tar.c b/archive-tar.c
index 33e7657..555850a 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -319,6 +319,13 @@ int write_tar_archive(struct archiver_args *args)
}
read_tree_recursive(args->tree, args->base, plen, 0,
args->pathspec, write_tar_entry);
+ if (args->commit_sha1 && args->commit_sha1_file) {
+ unsigned char fake_sha1[20];
+ pretend_sha1_file(sha1_to_hex(args->commit_sha1), 40,
+ OBJ_BLOB, fake_sha1);
+ write_tar_entry(fake_sha1, args->base, plen,
+ args->commit_sha1_file, 0100666, 0);
+ }
write_trailer();
return 0;
diff --git a/archive-zip.c b/archive-zip.c
index 3cbf6bb..88c5dfa 100644
--- a/archive-zip.c
+++ b/archive-zip.c
@@ -328,6 +328,13 @@ int write_zip_archive(struct archiver_args *args)
}
read_tree_recursive(args->tree, args->base, plen, 0,
args->pathspec, write_zip_entry);
+ if (args->commit_sha1 && args->commit_sha1_file) {
+ unsigned char fake_sha1[20];
+ pretend_sha1_file(sha1_to_hex(args->commit_sha1), 40,
+ OBJ_BLOB, fake_sha1);
+ write_zip_entry(fake_sha1, args->base, plen,
+ args->commit_sha1_file, 0100666, 0);
+ }
write_zip_trailer(args->commit_sha1);
free(zip_dir);
diff --git a/archive.h b/archive.h
index 6838dc7..020f82f 100644
--- a/archive.h
+++ b/archive.h
@@ -8,6 +8,7 @@ struct archiver_args {
const char *base;
struct tree *tree;
const unsigned char *commit_sha1;
+ const char *commit_sha1_file;
time_t time;
const char **pathspec;
unsigned int verbose : 1;
diff --git a/builtin-archive.c b/builtin-archive.c
index 7f4e409..6bb0781 100644
--- a/builtin-archive.c
+++ b/builtin-archive.c
@@ -151,6 +151,7 @@ int parse_archive_args(int argc, const char **argv, struct archiver *ar)
int extra_argc = 0;
const char *format = "tar";
const char *base = "";
+ const char *commit_sha1_file = NULL;
int verbose = 0;
int i;
@@ -174,6 +175,10 @@ int parse_archive_args(int argc, const char **argv, struct archiver *ar)
base = arg + 9;
continue;
}
+ if (!prefixcmp(arg, "--commit-id-file=")) {
+ commit_sha1_file = arg + 17;
+ continue;
+ }
if (!strcmp(arg, "--")) {
i++;
break;
@@ -192,6 +197,11 @@ int parse_archive_args(int argc, const char **argv, struct archiver *ar)
usage(archive_usage);
if (init_archiver(format, ar) < 0)
die("Unknown archive format '%s'", format);
+ if (commit_sha1_file) {
+ size_t namelen = strlen(commit_sha1_file);
+ if (namelen == 0 || commit_sha1_file[namelen - 1] == '/')
+ die("Invalid commit ID file name: %s", commit_sha1_file);
+ }
if (extra_argc) {
if (!ar->parse_extra)
@@ -201,6 +211,7 @@ int parse_archive_args(int argc, const char **argv, struct archiver *ar)
}
ar->args.verbose = verbose;
ar->args.base = base;
+ ar->args.commit_sha1_file = commit_sha1_file;
return i;
}
@@ -236,6 +247,20 @@ static const char *extract_remote_arg(int *ac, const char **av)
return remote;
}
+static int is_path_in_spec(struct tree *tree, const char **pathspec,
+ const char *path)
+{
+ unsigned char sha1[20];
+ unsigned int mode;
+
+ if (get_tree_entry(tree->object.sha1, path, sha1, &mode))
+ return 0;
+ if (!pathspec)
+ return 1;
+ /* TODO: the actual pathspec check */
+ return 1;
+}
+
int cmd_archive(int argc, const char **argv, const char *prefix)
{
struct archiver ar;
@@ -257,5 +282,12 @@ int cmd_archive(int argc, const char **argv, const char *prefix)
parse_treeish_arg(argv, &ar.args, prefix);
parse_pathspec_arg(argv + 1, &ar.args);
+ if (ar.args.commit_sha1_file) {
+ if (is_path_in_spec(ar.args.tree, ar.args.pathspec, ar.args.commit_sha1_file))
+ die("Commit ID file name already exists in archive.");
+ if (!ar.args.commit_sha1)
+ die("Need a commit to use --commit-id-file, and not a tree.");
+ }
+
return ar.write_archive(&ar.args);
}
^ permalink raw reply related
* [PATCH] git-applymbox: Remove command
From: Petr Baudis @ 2007-05-20 0:10 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
I believe noone uses git-applymbox, and noone definitely should, since it
is supposed to be completely superseded and everything by its younger
cousin git-am. The only known person in the universe to use it was Linus
and he declared some time ago that he will try to use git-am instead in his
famous dotest script.
The trouble is that git-applymbox existence creates confusing UI. I'm a bit
like a recycled newbie to the git porcelain and *I* was confused by
git-applymbox primitiveness until I've realized a while later that I'm of
course using the wrong command.
Signed-off-by: Petr Baudis <pasky@suse.cz>
---
(Admittelly this patch is a bit brave and maybe we should make applymbox
print some scary messages first for some time. I'm testing the water here
to see if anyone complains. I still think that noone really uses it,
though.)
---
.gitignore | 1
Documentation/cmd-list.perl | 1
Documentation/git-am.txt | 5 +-
Documentation/git-applymbox.txt | 98 --------------------------------
Documentation/hooks.txt | 6 +-
Makefile | 2 -
git-applymbox.sh | 121 ---------------------------------------
7 files changed, 6 insertions(+), 228 deletions(-)
diff --git a/.gitignore b/.gitignore
index d0b67da..16f4aba 100644
--- a/.gitignore
+++ b/.gitignore
@@ -7,7 +7,6 @@ git-add--interactive
git-am
git-annotate
git-apply
-git-applymbox
git-applypatch
git-archimport
git-archive
diff --git a/Documentation/cmd-list.perl b/Documentation/cmd-list.perl
index 443802a..0bca346 100755
--- a/Documentation/cmd-list.perl
+++ b/Documentation/cmd-list.perl
@@ -72,7 +72,6 @@ __DATA__
git-add mainporcelain
git-am mainporcelain
git-annotate ancillaryinterrogators
-git-applymbox ancillaryinterrogators
git-applypatch purehelpers
git-apply plumbingmanipulators
git-archimport foreignscminterface
diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt
index ba79773..bf07e43 100644
--- a/Documentation/git-am.txt
+++ b/Documentation/git-am.txt
@@ -126,8 +126,7 @@ is terminated before the first occurrence of such a line.
When initially invoking it, you give it names of the mailboxes
to crunch. Upon seeing the first patch that does not apply, it
-aborts in the middle, just like 'git-applymbox' does. You can
-recover from this in one of two ways:
+aborts in the middle,. You can recover from this in one of two ways:
. skip the current patch by re-running the command with '--skip'
option.
@@ -144,7 +143,7 @@ names.
SEE ALSO
--------
-gitlink:git-applymbox[1], gitlink:git-applypatch[1], gitlink:git-apply[1].
+gitlink:git-applypatch[1], gitlink:git-apply[1].
Author
diff --git a/Documentation/git-applymbox.txt b/Documentation/git-applymbox.txt
deleted file mode 100644
index ea919ba..0000000
--- a/Documentation/git-applymbox.txt
+++ /dev/null
@@ -1,98 +0,0 @@
-git-applymbox(1)
-================
-
-NAME
-----
-git-applymbox - Apply a series of patches in a mailbox
-
-
-SYNOPSIS
---------
-'git-applymbox' [-u] [-k] [-q] [-m] ( -c .dotest/<num> | <mbox> ) [ <signoff> ]
-
-DESCRIPTION
------------
-Splits mail messages in a mailbox into commit log message,
-authorship information and patches, and applies them to the
-current branch.
-
-
-OPTIONS
--------
--q::
- Apply patches interactively. The user will be given
- opportunity to edit the log message and the patch before
- attempting to apply it.
-
--k::
- Usually the program 'cleans up' the Subject: header line
- to extract the title line for the commit log message,
- among which (1) remove 'Re:' or 're:', (2) leading
- whitespaces, (3) '[' up to ']', typically '[PATCH]', and
- then prepends "[PATCH] ". This flag forbids this
- munging, and is most useful when used to read back 'git
- format-patch -k' output.
-
--m::
- Patches are applied with `git-apply` command, and unless
- it cleanly applies without fuzz, the processing fails.
- With this flag, if a tree that the patch applies cleanly
- is found in a repository, the patch is applied to the
- tree and then a 3-way merge between the resulting tree
- and the current tree.
-
--u::
- Pass `-u` flag to `git-mailinfo` (see gitlink:git-mailinfo[1]).
- The proposed commit log message taken from the e-mail
- are re-coded into UTF-8 encoding (configuration variable
- `i18n.commitencoding` can be used to specify project's
- preferred encoding if it is not UTF-8). This used to be
- optional but now it is the default.
-+
-Note that the patch is always used as-is without charset
-conversion, even with this flag.
-
--n::
- Pass `-n` flag to `git-mailinfo` (see
- gitlink:git-mailinfo[1]).
-
--c .dotest/<num>::
- When the patch contained in an e-mail does not cleanly
- apply, the command exits with an error message. The
- patch and extracted message are found in .dotest/, and
- you could re-run 'git applymbox' with '-c .dotest/<num>'
- flag to restart the process after inspecting and fixing
- them.
-
-<mbox>::
- The name of the file that contains the e-mail messages
- with patches. This file should be in the UNIX mailbox
- format. See 'SubmittingPatches' document to learn about
- the formatting convention for e-mail submission.
-
-<signoff>::
- The name of the file that contains your "Signed-off-by"
- line. See 'SubmittingPatches' document to learn what
- "Signed-off-by" line means. You can also just say
- 'yes', 'true', 'me', or 'please' to use an automatically
- generated "Signed-off-by" line based on your committer
- identity.
-
-
-SEE ALSO
---------
-gitlink:git-am[1], gitlink:git-applypatch[1].
-
-
-Author
-------
-Written by Linus Torvalds <torvalds@osdl.org>
-
-Documentation
---------------
-Documentation by Junio C Hamano and the git-list <git@vger.kernel.org>.
-
-GIT
----
-Part of the gitlink:git[7] suite
-
diff --git a/Documentation/hooks.txt b/Documentation/hooks.txt
index aabb975..aad1744 100644
--- a/Documentation/hooks.txt
+++ b/Documentation/hooks.txt
@@ -13,7 +13,7 @@ applypatch-msg
--------------
This hook is invoked by `git-applypatch` script, which is
-typically invoked by `git-applymbox`. It takes a single
+typically invoked by `git-am`. It takes a single
parameter, the name of the file that holds the proposed commit
log message. Exiting with non-zero status causes
`git-applypatch` to abort before applying the patch.
@@ -30,7 +30,7 @@ pre-applypatch
--------------
This hook is invoked by `git-applypatch` script, which is
-typically invoked by `git-applymbox`. It takes no parameter,
+typically invoked by `git-am`. It takes no parameter,
and is invoked after the patch is applied, but before a commit
is made. Exiting with non-zero status causes the working tree
after application of the patch not committed.
@@ -45,7 +45,7 @@ post-applypatch
---------------
This hook is invoked by `git-applypatch` script, which is
-typically invoked by `git-applymbox`. It takes no parameter,
+typically invoked by `git-am`. It takes no parameter,
and is invoked after the patch is applied and a commit is made.
This hook is meant primarily for notification, and cannot affect
diff --git a/Makefile b/Makefile
index de74851..77d3ad0 100644
--- a/Makefile
+++ b/Makefile
@@ -208,7 +208,7 @@ SCRIPT_SH = \
git-repack.sh git-request-pull.sh git-reset.sh \
git-sh-setup.sh \
git-tag.sh git-verify-tag.sh \
- git-applymbox.sh git-applypatch.sh git-am.sh \
+ git-applypatch.sh git-am.sh \
git-merge.sh git-merge-stupid.sh git-merge-octopus.sh \
git-merge-resolve.sh git-merge-ours.sh \
git-lost-found.sh git-quiltimport.sh
diff --git a/git-applymbox.sh b/git-applymbox.sh
deleted file mode 100755
index c18e80f..0000000
--- a/git-applymbox.sh
+++ /dev/null
@@ -1,121 +0,0 @@
-#!/bin/sh
-##
-## "dotest" is my stupid name for my patch-application script, which
-## I never got around to renaming after I tested it. We're now on the
-## second generation of scripts, still called "dotest".
-##
-## Update: Ryan Anderson finally shamed me into naming this "applymbox".
-##
-## You give it a mbox-format collection of emails, and it will try to
-## apply them to the kernel using "applypatch"
-##
-## The patch application may fail in the middle. In which case:
-## (1) look at .dotest/patch and fix it up to apply
-## (2) re-run applymbox with -c .dotest/msg-number for the current one.
-## Pay a special attention to the commit log message if you do this and
-## use a Signoff_file, because applypatch wants to append the sign-off
-## message to msg-clean every time it is run.
-##
-## git-am is supposed to be the newer and better tool for this job.
-
-USAGE='[-u] [-k] [-q] [-m] (-c .dotest/<num> | mbox) [signoff]'
-. git-sh-setup
-
-git var GIT_COMMITTER_IDENT >/dev/null || exit
-
-keep_subject= query_apply= continue= utf8=-u resume=t
-while case "$#" in 0) break ;; esac
-do
- case "$1" in
- -u) utf8=-u ;;
- -n) utf8=-n ;;
- -k) keep_subject=-k ;;
- -q) query_apply=t ;;
- -c) continue="$2"; resume=f; shift ;;
- -m) fall_back_3way=t ;;
- -*) usage ;;
- *) break ;;
- esac
- shift
-done
-
-case "$continue" in
-'')
- rm -rf .dotest
- mkdir .dotest
- num_msgs=$(git-mailsplit "$1" .dotest) || exit 1
- echo "$num_msgs patch(es) to process."
- shift
-esac
-
-files=$(git-diff-index --cached --name-only HEAD) || exit
-if [ "$files" ]; then
- echo "Dirty index: cannot apply patches (dirty: $files)" >&2
- exit 1
-fi
-
-case "$query_apply" in
-t) touch .dotest/.query_apply
-esac
-case "$fall_back_3way" in
-t) : >.dotest/.3way
-esac
-case "$keep_subject" in
--k) : >.dotest/.keep_subject
-esac
-
-signoff="$1"
-set x .dotest/0*
-shift
-while case "$#" in 0) break;; esac
-do
- i="$1"
- case "$resume,$continue" in
- f,$i) resume=t;;
- f,*) shift
- continue;;
- *)
- git-mailinfo $keep_subject $utf8 \
- .dotest/msg .dotest/patch <$i >.dotest/info || exit 1
- test -s .dotest/patch || {
- echo "Patch is empty. Was it split wrong?"
- exit 1
- }
- git-stripspace < .dotest/msg > .dotest/msg-clean
- ;;
- esac
- while :; # for fixing up and retry
- do
- git-applypatch .dotest/msg-clean .dotest/patch .dotest/info "$signoff"
- case "$?" in
- 0)
- # Remove the cleanly applied one to reduce clutter.
- rm -f .dotest/$i
- ;;
- 2)
- # 2 is a special exit code from applypatch to indicate that
- # the patch wasn't applied, but continue anyway
- ;;
- *)
- ret=$?
- if test -f .dotest/.query_apply
- then
- echo >&2 "* Patch failed."
- echo >&2 "* You could fix it up in your editor and"
- echo >&2 " retry. If you want to do so, say yes here"
- echo >&2 " AFTER fixing .dotest/patch up."
- echo >&2 -n "Retry [y/N]? "
- read yesno
- case "$yesno" in
- [Yy]*)
- continue ;;
- esac
- fi
- exit $ret
- esac
- break
- done
- shift
-done
-# return to pristine
-rm -fr .dotest
^ permalink raw reply related
* Re: [PATCH 2/2] Teach "delta" attribute to pack-objects.
From: Dana How @ 2007-05-19 23:56 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, danahow
In-Reply-To: <7v4pm8wpgc.fsf@assigned-by-dhcp.cox.net>
On 5/19/07, Junio C Hamano <junkio@cox.net> wrote:
> "Dana How" <danahow@gmail.com> writes:
> > ... But such an attribute should be
> > ignored when --stdout is in effect -- it only affects on-disk
> > repacking, not packing for transfers, which is why it's named "repack".
>
> Yes -- if we want to have an option to keep objects selectively
> left out of packs in loose format, you would need 'repack' which
> acts differently between the server-feeding-client case vs
> packing-repository case.
>
> Which is a bigger change that I did not want to show in the
> quick-and-clean patch, but I would agree we would want both.
OK -- I'll put this on the back-burner with the so-called
"ent:relative" patch and I'll revisit this once your "delta"
attribute shows up in next or master.
At the moment I'm experimenting on a git repository with
a 4.5GB checkout, and 18 months of history in 4K commits
comprising 100GB (uncompressed) of blobs stored in
7 packfiles of 2GB or less. Hopefully I'll be able to say
more about tweaking packing shortly.
Thanks,
--
Dana L. How danahow@gmail.com +1 650 804 5991 cell
^ permalink raw reply
* Re: Using git to store /etc, redux
From: david @ 2007-05-19 23:37 UTC (permalink / raw)
To: David Härdeman; +Cc: git
In-Reply-To: <20070519174815.GA5124@hardeman.nu>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 2001 bytes --]
On Sat, 19 May 2007, David Härdeman wrote:
> I recently had the idea to store and track /etc using git. When googling the
> topic I came across the "Using git to store /etc" thread from the end of last
> year which provided some interesting details on what would be necessary.
>
> It seems the file metadata (owner, group, mode, xattrs, etc) was the big
> stumbling point, so I wrote up a tool over the last few days which allows the
> metadata to be stored in a separate file which can be stored along with the
> rest of the data in the repo (or separately).
>
> This is also useful for tripwire type checks and for other types of storage
> which drops some of the metadata (tar comes to mind)...
>
> The tool (metastore) is available from: git://git.hardeman.nu/metastore.git
>
> Not completely cleaned up yet (it lacks a real README and some Makefile
> targets) but I hope it might be useful to others (it sure is to me).
>
> Please CC me on any replies.
as I understand the issue, the problem isn't creating a tool to store the
metadata, but in integrating things with git.
when checking something in a pre-commit hook needs to run the tool to
store the data.
git supports this and it's pretty simple to do this.
however when checking things out there are approaches
1. modify git to have a post-checkout hook to set the metadata to match
what was stored at checkin and accept the fact that this leaves a
window where the file has the wrong metadata on it (between when the
file is written and when the hook runs), or use a staging area to have
copies of the files during check-in and check-out
2. modify git to know that it needs to check some files out before any
others and use an expernal program to write the files to disk. Then
this program can use the data stored at checkin to write the files with
the appropriate metadata
unforutnantly until one of these is done by someone the utility of
programs like your metastore are limited.
David Lang
^ permalink raw reply
* Re: [PATCH] Teach 'git-apply --whitespace=strip' to remove empty lines at end of file
From: Junio C Hamano @ 2007-05-19 23:18 UTC (permalink / raw)
To: Marco Costalba; +Cc: Git Mailing List
In-Reply-To: <7vsl9stnzp.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano <junkio@cox.net> writes:
> ... these may or may not be at the end of the file, so
> inspecting what blank lines they have at the end is not
> sufficient. If "new" does not introduce new blank lines at its
> end, then you can be sure that you are not adding trailing blank
> lines, but even if "new" does introduce a new blank line at the
> end, you do not know if that is adding it to the end of the
> file, or in the middle.
>
> You do not know where the hunk is applied until you do the loop
> that follows the part your patch we are discussing.
If I were doing this, I would probably do it this way:
(1) Inside apply_one_fragment(), where "case '+':" appears, count
the blank (not just '\n', but matches /^\s*$/) lines at the
end of "new" side. As soon as you fall into "case ' ':" or
"case '-':" or non-blank line in "case '+':", you reset the
counter to zero, so that what you are counting is the
number of blank lines that would have get added, if the
hunk were to be applied at the end of the file. Keep that
number ofter you separated the fragment into new and old.
(2) In the same function, inside the big "for (;;)" loop that
figures out where to apply that "old" => "new" change, use
the number you gathered in the step (1) to trim what is
applied, where the real application happens, which is the
part that has memmove()/memcpy(), only when you know you
are applying the hunk at the end of the file. That is the
only place in the function that knows where the hunk is
being applied.
^ permalink raw reply
* [PATCH v2] Add an option to git-ls-tree to display also the size of blob
From: Jakub Narebski @ 2007-05-19 20:08 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Shawn Pearce
In-Reply-To: <7vps51hacw.fsf@assigned-by-dhcp.cox.net>
Add -l/--long option to git-ls-tree command, which displays
object size of a blob entry. Object size is placed after
object id (left-justified with minimum width of 7 characters).
For non-blob entries `-' is used.
Rationale: for non-blob entries size of an object has no much
meaning, and is not very interesting. Moreover, in planned
pack v4 tree objects would be constructed on demand, so tree
size would need to be calculated... although isn't object size
stored in the header?
While at it, cleanup whitespace: tabs are for indent, spaces are
for align.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
I hope this addresses concerns mentioned in this thread: the
alternate name --size for -l/--long option, and showing size
for tree (and commit/submodule) objects.
Documentation/git-ls-tree.txt | 14 +++++++++++++-
builtin-ls-tree.c | 39 +++++++++++++++++++++++++++++++--------
2 files changed, 44 insertions(+), 9 deletions(-)
diff --git a/Documentation/git-ls-tree.txt b/Documentation/git-ls-tree.txt
index 7899394..ad7f1b9 100644
--- a/Documentation/git-ls-tree.txt
+++ b/Documentation/git-ls-tree.txt
@@ -9,7 +9,7 @@ git-ls-tree - List the contents of a tree object
SYNOPSIS
--------
[verse]
-'git-ls-tree' [-d] [-r] [-t] [-z]
+'git-ls-tree' [-d] [-r] [-t] [-l] [-z]
[--name-only] [--name-status] [--full-name] [--abbrev=[<n>]]
<tree-ish> [paths...]
@@ -36,6 +36,10 @@ OPTIONS
Show tree entries even when going to recurse them. Has no effect
if '-r' was not passed. '-d' implies '-t'.
+-l::
+--long::
+ Show object size of blob (file) entries.
+
-z::
\0 line termination on output.
@@ -65,6 +69,14 @@ Output Format
When the `-z` option is not used, TAB, LF, and backslash characters
in pathnames are represented as `\t`, `\n`, and `\\`, respectively.
+When the `-l` option is used, format changes to
+
+ <mode> SP <type> SP <object> SP <object size> TAB <file>
+
+Object size identified by <object> is given in bytes, and right-justified
+with minimum width of 7 characters. Object size is given only for blobs
+(file) entries; for other entries `-` character is used in place of size.
+
Author
------
diff --git a/builtin-ls-tree.c b/builtin-ls-tree.c
index 1cb4dca..1d2dc40 100644
--- a/builtin-ls-tree.c
+++ b/builtin-ls-tree.c
@@ -15,6 +15,7 @@ static int line_termination = '\n';
#define LS_TREE_ONLY 2
#define LS_SHOW_TREES 4
#define LS_NAME_ONLY 8
+#define LS_SHOW_SIZE 16
static int abbrev;
static int ls_options;
static const char **pathspec;
@@ -22,7 +23,7 @@ static int chomp_prefix;
static const char *ls_tree_prefix;
static const char ls_tree_usage[] =
- "git-ls-tree [-d] [-r] [-t] [-z] [--name-only] [--name-status] [--full-name] [--abbrev[=<n>]] <tree-ish> [path...]";
+ "git-ls-tree [-d] [-r] [-t] [-l] [-z] [--name-only] [--name-status] [--full-name] [--abbrev[=<n>]] <tree-ish> [path...]";
static int show_recursive(const char *base, int baselen, const char *pathname)
{
@@ -55,10 +56,11 @@ static int show_recursive(const char *base, int baselen, const char *pathname)
}
static int show_tree(const unsigned char *sha1, const char *base, int baselen,
- const char *pathname, unsigned mode, int stage)
+ const char *pathname, unsigned mode, int stage)
{
int retval = 0;
const char *type = blob_type;
+ unsigned long size;
if (S_ISDIRLNK(mode)) {
/*
@@ -92,13 +94,27 @@ static int show_tree(const unsigned char *sha1, const char *base, int baselen,
(baselen < chomp_prefix || memcmp(ls_tree_prefix, base, chomp_prefix)))
return 0;
- if (!(ls_options & LS_NAME_ONLY))
- printf("%06o %s %s\t", mode, type,
- abbrev ? find_unique_abbrev(sha1,abbrev)
- : sha1_to_hex(sha1));
+ if (!(ls_options & LS_NAME_ONLY)) {
+ if (ls_options & LS_SHOW_SIZE) {
+ if (!strcmp(type, blob_type)) {
+ sha1_object_info(sha1, &size);
+ printf("%06o %s %s %7lu\t", mode, type,
+ abbrev ? find_unique_abbrev(sha1, abbrev)
+ : sha1_to_hex(sha1),
+ size);
+ } else
+ printf("%06o %s %s %7c\t", mode, type,
+ abbrev ? find_unique_abbrev(sha1, abbrev)
+ : sha1_to_hex(sha1),
+ '-');
+ } else
+ printf("%06o %s %s\t", mode, type,
+ abbrev ? find_unique_abbrev(sha1, abbrev)
+ : sha1_to_hex(sha1));
+ }
write_name_quoted(base + chomp_prefix, baselen - chomp_prefix,
- pathname,
- line_termination, stdout);
+ pathname,
+ line_termination, stdout);
putchar(line_termination);
return retval;
}
@@ -126,12 +142,19 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)
case 't':
ls_options |= LS_SHOW_TREES;
break;
+ case 'l':
+ ls_options |= LS_SHOW_SIZE;
+ break;
case '-':
if (!strcmp(argv[1]+2, "name-only") ||
!strcmp(argv[1]+2, "name-status")) {
ls_options |= LS_NAME_ONLY;
break;
}
+ if (!strcmp(argv[1]+2, "long")) {
+ ls_options |= LS_SHOW_SIZE;
+ break;
+ }
if (!strcmp(argv[1]+2, "full-name")) {
chomp_prefix = 0;
break;
--
1.5.1.4
^ permalink raw reply related
* Re: [3/4] What's not in 1.5.2 (new topics)
From: Jakub Narebski @ 2007-05-19 16:38 UTC (permalink / raw)
To: Petr Baudis
Cc: Josef Weidendorfer, Michael S. Tsirkin, Junio C Hamano,
Andy Parkins, Nicolas Pitre, git
In-Reply-To: <20070518124123.GX4489@pasky.or.cz>
On Fri, 18 May 2007, Petr Baudis wrote:
> On Fri, May 18, 2007 at 02:00:07PM CEST, Jakub Narebski wrote:
>> There is a bit ugly solution for this: instead of using symbolic name
>> in versioned .gitmodules for a subproject (for a repo), use subproject
>> identifier (inode), and put it in the tag object (or config) together with
>> the URL. Git would then search all the subproject / submodule info for
>> a given inode. You could have more than one inode / identifier name for
>> a subproject repo; this would avoid the "independently created" issue
>> with using inodes / file-ids in distributed SCM. One would have to
>> ensure however that different subprojects get assigned different inodes.
>
> Well, then it doesn't make any difference, no? You just renamed the
> problem but it stays the same - to ensure uniqueness even across
> repositories.
>
> Ok, you can declare now that you will just think out a UUID for the
> subproject, but aside of not fitting well with the whole git philosophy,
> then you don't need the indirection again, just use the UUID as the tag
> name.
>
> I have the feeling that I'm missing something basic in your proposal...
I was thinking about _automatic_ UUID, generated by git. For example it
could be sha1 of first subproject commit which appeared in supermodule.
It is easy to check if two UUID correspond to the same repository:
check if both objects are present, or perhaps that one is reachable from
the other, or that they have common parent. This kind of UUID is not
that different from (global) SHA1 of object.
So the idea is to have versioned, i.e. in-tree mapping from directory
names to repositories via some kind of identifier: Junio idea of using
URL of repository, with possibility of overriding it in repo config,
the idea of using tag name, and having URL for repo in tag contents,
and my idea of tag name of tag containing UUID. To find the URL you
would search all the repo-tags for UUID, or for existence of commit
with given sha1.
But I haven't thought this idea through, so it migh be utter rubbish.
The porcelain part of subproject / submodule support is not that
easy, to cover for moving subproject "mountpoint", project changing URL,
conflict of project names and different naming of the same project etc.
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCH] Teach 'git-apply --whitespace=strip' to remove empty lines at end of file
From: Junio C Hamano @ 2007-05-19 23:03 UTC (permalink / raw)
To: Marco Costalba; +Cc: Git Mailing List
In-Reply-To: <e5bfff550705191458o3fa1d1a0ndd916d1efafdd2a7@mail.gmail.com>
"Marco Costalba" <mcostalba@gmail.com> writes:
> What about this?
>
> builtin-apply.c | 13 +++++++++----
> 1 files changed, 9 insertions(+), 4 deletions(-)
You count the trailing blank lines in new and old, and if new
one has more you strip them out, which _sounds_ sane.
But it is unclear to me how you are limiting the processing to
the very end of the file. The "new" and "old" essentially is a
patch fragment that is separated into two, and the part you
modified with your patch does not know if the hunk applies at
the end of the patch yet.
That is, given this patch:
diff --git a/builtin-apply.c b/builtin-apply.c
index 9e82757..113c71f 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -1746,10 +1746,15 @@ static int apply_one_fragment(struct buffer_desc *des..
newsize--;
}
- if (new_whitespace == strip_whitespace)
- while (newsize > 1 && !strncmp(new + newsize - 2, "\n\n", 2))
- newsize--;
-
+ if (new_whitespace == strip_whitespace) {
+ int cnt1 = 1, cnt2 = 1;
+ while (newsize - cnt1 > 1 && new[newsize - cnt1] == '\n')
+ cnt1++;
+ while (oldsize - cnt2 > 1 && new[newsize - cnt2] == '\n')
+ cnt2++;
+ if (cnt1 > cnt2 && cnt1 > 2)
+ newsize -= cnt1 - cnt2;
+ }
oldlines = old;
newlines = new;
leading = frag->leading;
"new" has these lines
newsize--;
}
if (new_whitespace == strip_whitespace) {
int cnt1 = 1, cnt2 = 1;
while (newsize - cnt1 > 1 && new[newsize - cnt1] == '\n')
cnt1++;
while (oldsize - cnt2 > 1 && new[newsize - cnt2] == '\n')
cnt2++;
if (cnt1 > cnt2 && cnt1 > 2)
newsize -= cnt1 - cnt2;
}
oldlines = old;
newlines = new;
leading = frag->leading;
while "old" has this:
newsize--;
}
if (new_whitespace == strip_whitespace)
while (newsize > 1 && !strncmp(new + newsize - 2, "\n\n", 2))
newsize--;
oldlines = old;
newlines = new;
leading = frag->leading;
and these may or may not be at the end of the file, so
inspecting what blank lines they have at the end is not
sufficient. If "new" does not introduce new blank lines at its
end, then you can be sure that you are not adding trailing blank
lines, but even if "new" does introduce a new blank line at the
end, you do not know if that is adding it to the end of the
file, or in the middle.
You do not know where the hunk is applied until you do the loop
that follows the part your patch we are discussing.
^ permalink raw reply related
* Re: [PATCH] Teach 'git-apply --whitespace=strip' to remove empty lines at end of file
From: Marco Costalba @ 2007-05-19 21:58 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <7vhcq8v97k.fsf@assigned-by-dhcp.cox.net>
On 5/19/07, Junio C Hamano <junkio@cox.net> wrote:
> Marco Costalba <mcostalba@gmail.com> writes:
>
> > Signed-off-by: Marco Costalba <mcostalba@gmail.com>
> > ---
> > builtin-apply.c | 4 ++++
> > 1 files changed, 4 insertions(+), 0 deletions(-)
> >
> > diff --git a/builtin-apply.c b/builtin-apply.c
> > index 0399743..f17f838 100644
> > --- a/builtin-apply.c
> > +++ b/builtin-apply.c
> > @@ -1738,6 +1738,10 @@ static int apply_one_fragment(struct buffer_desc *desc, struct fragment *frag, i
> > newsize--;
> > }
> >
> > + if (new_whitespace == strip_whitespace)
> > + while (newsize > 1 && !strncmp(new + newsize - 2, "\n\n", 2))
> > + newsize--;
> > +
> > oldlines = old;
> > newlines = new;
> > leading = frag->leading;
>
> I agree to what you are trying to do, but this patch is wrong.
> You are stripping trailing newlines that were NOT introduced by
> the patch, but happened to be present in the preimage (and in
> the context).
>
> Try it on this test vector:
>
> cat >AAA <<\EOF
> a
> b
> c
> d
>
>
> e
> f
> g
> h
> i
>
>
> j
> k
>
>
> EOF
> cat >P.diff <<\EOF
> diff --git a/AAA b/AAA
> index 59f6a9c..ffb28f5 100644
> --- a/AAA
> +++ b/AAA
> @@ -1,4 +1,4 @@
> -a
> +A
> b
> c
> d
> @@ -6,12 +6,11 @@ d
>
> e
> f
> +
> +
> g
> h
> -i
> -
>
> -j
> k
>
>
> EOF
>
>
What about this?
builtin-apply.c | 13 +++++++++----
1 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/builtin-apply.c b/builtin-apply.c
index 9e82757..113c71f 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -1746,10 +1746,15 @@ static int apply_one_fragment(struct
buffer_desc *desc, struct fragment *frag, i
newsize--;
}
- if (new_whitespace == strip_whitespace)
- while (newsize > 1 && !strncmp(new + newsize - 2, "\n\n", 2))
- newsize--;
-
+ if (new_whitespace == strip_whitespace) {
+ int cnt1 = 1, cnt2 = 1;
+ while (newsize - cnt1 > 1 && new[newsize - cnt1] == '\n')
+ cnt1++;
+ while (oldsize - cnt2 > 1 && new[newsize - cnt2] == '\n')
+ cnt2++;
+ if (cnt1 > cnt2 && cnt1 > 2)
+ newsize -= cnt1 - cnt2;
+ }
oldlines = old;
newlines = new;
leading = frag->leading;
^ permalink raw reply related
* Re: Commit ID in exported Tar Ball
From: A Large Angry SCM @ 2007-05-19 21:39 UTC (permalink / raw)
To: Junio C Hamano
Cc: René Scharfe, Frank Lichtenheld, Johan Herland, git,
Thomas Glanzmann, Michael Gernoth
In-Reply-To: <7vd50wv88t.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano wrote:
> René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
>
>> Something like the following patch? Since we're already embedding the
>> commit ID in a comment, we might as well offer creating a synthetic file
>> for it, too, if that solves a user's problem that might be difficult to
>> work around otherwise.
>
> Are you detecting and erroring out if the named file already
> exists in the tree being archived?
>
> Should we? Maybe we should just replace with warning?
>
> Also should we silently ignore the request if the tree-ish is
> not a commit-ish, or error out?
Silently ignoring conflicting command arguments is (almost always) a
bug; either in the implementation or the design.
^ permalink raw reply
* [PATCH] Update bash completion to ignore some more plumbing commands
From: Jonas Fonseca @ 2007-05-19 21:35 UTC (permalink / raw)
To: git, Junio C Hamano; +Cc: Shawn O. Pearce
Signed-off-by: Jonas Fonseca <fonseca@diku.dk>
---
contrib/completion/git-completion.bash | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
I am not sure if it is OK to exclude checkout-index as it is
the only method of exporting the source as a directory. Also,
I don't understand why git-diff-* is not excluded.
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 46356e8..e758242 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -262,7 +262,9 @@ __git_commands ()
applypatch) : ask gittus;;
archimport) : import;;
cat-file) : plumbing;;
+ check-attr) : plumbing;;
check-ref-format) : plumbing;;
+ checkout-index) : plumbing;;
commit-tree) : plumbing;;
convert-objects) : plumbing;;
cvsexportcommit) : export;;
@@ -271,8 +273,10 @@ __git_commands ()
daemon) : daemon;;
fast-import) : import;;
fsck-objects) : plumbing;;
+ fetch--tool) : plumbing;;
fetch-pack) : plumbing;;
fmt-merge-msg) : plumbing;;
+ for-each-ref) : plumbing;;
hash-object) : plumbing;;
http-*) : transport;;
index-pack) : plumbing;;
--
1.5.2.rc3.800.ga489e-dirty
--
Jonas Fonseca
^ permalink raw reply related
* Re: Commit ID in exported Tar Ball
From: Junio C Hamano @ 2007-05-19 21:00 UTC (permalink / raw)
To: René Scharfe
Cc: Frank Lichtenheld, Johan Herland, git, Thomas Glanzmann,
Michael Gernoth
In-Reply-To: <464F5CA2.3070809@lsrfire.ath.cx>
René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
> Something like the following patch? Since we're already embedding the
> commit ID in a comment, we might as well offer creating a synthetic file
> for it, too, if that solves a user's problem that might be difficult to
> work around otherwise.
Are you detecting and erroring out if the named file already
exists in the tree being archived?
Should we? Maybe we should just replace with warning?
Also should we silently ignore the request if the tree-ish is
not a commit-ish, or error out?
^ 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