* Re: [PATCH] Rearrange git-format-patch synopsis to improve clarity.
From: David Symonds @ 2007-11-06 21:08 UTC (permalink / raw)
To: Jon Loeliger; +Cc: Junio C Hamano, git
In-Reply-To: <473081C2.7060106@freescale.com>
On 11/7/07, Jon Loeliger <jdl@freescale.com> wrote:
> David Symonds wrote:
> > Now that I look at it again, it seems the long options look quite
> > inconsistent. I think it should be either
> > --numbered-files/--no-numbered-files or --numbered/--no-numbered. My
> > preference is with the latter (for brevity), but that breaks
> > backward-compatibility.
> >
> > Would you accept a patch that changed --numbered-files to --numbered,
> > and kept the former as a synonym?
>
> There are two forms of numbered file output names:
> the traditional "0001-Foo-the-bar" and just "1" styles.
> Please don't break that. Both are needed.
Oh, I certainly wasn't proposing removing any functionality; merely
renaming the option to select it so as to be consistent.
Dave.
^ permalink raw reply
* Re: [PATCH 0/5] some shell portability fixes
From: Ralf Wildenhues @ 2007-11-06 21:09 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v8x5bgl04.fsf@gitster.siamese.dyndns.org>
Hello Junio,
* Junio C Hamano wrote on Tue, Nov 06, 2007 at 09:46:35PM CET:
> All missing Signed-off-by: lines.
Oops. Sorry.
> [1/5] In addition to take advantage of the fact that the RHS of
> assignment is not split, I'd prefer replacing `` with $()
> with these cases. Much easier to read if your shell
> supports it (and all the modern ones do).
OK.
> [2/5] Gaah, AIX sed X-<. I am not opposed to this patch but
> would want to get Yays from people with non GNU sed. Is
> busybox sed good enough to grok our scripts these days?
> Please ask help and collect Acks at least from folks on
> Solaris, MacOS, FBSD, and OBSD.
FWIW, I have little experience with busybox sed, but for the others here
you go: With
echo axbyc | sed 's,x,\n,; s,y,\
,'
I get on OpenBSD, FreeBSD, Solaris, and Darwin (minus indentation):
anb
c
GNU sed gives
a
b
c
> [3/5] Arithmetic expansion. Have you caught _all_ of them, or
> is this patch about only the ones you noticed?
I have grepped *.sh. But let's drop that, I see that it goes backwards.
> [4/5] I wonder if use of fgrep would be easier to read and more
> portable with this one:
>
> name=$( GIT_CONFIG=.gitmodules \
> git config --get-regexp '^submodule\..*\.path$' |
> fgrep "submodule.$1.path" |
> sed -e 's/^submodule\.\(.*\)\.path$/\1/'
> )
Certainly easier to read. But fgrep itself is not portable (it could be
grep -F). Also, isn't the $1 to be matched at the end, after a "="
here? FWIW the pattern I posted has survived a few years in Automake,
so there is some hope that it works.
> [5/5] Again, have you covered all of them?
No, oops again. As I searched for `test.*-[oa]' I have missed line
wraps and [ ... -o ... ].
> I am not opposed to
> this one, although I am a bit curious who lacks -a/-o in
> practice.
Hmm, good question. I actually don't know whether there is a shell
that isn't ruled out by $() anyway. Let's drop that one, too, then.
Cheers,
Ralf
^ permalink raw reply
* git-bundle questions
From: Matthew Booth @ 2007-11-06 20:54 UTC (permalink / raw)
To: git
First a question: when creating a git bundle, is it possible to include
tags?
Secondly, has anybody given any thought to allowing a user to create a
bundle from any of the web front-ends? The reason for this request is
that I've been doing some work for a company where only corporate
standard desktops have internet access. They run Windows, and you can't
install additional software on them. This makes interacting with an
external git repository from my development servers problematic. Git
bundles do at least make this possible, but I have to create a bundle on
my laptop when I'm at home and transfer it to a server when I get to the
office. This would be a whole lot easier if I could just download the
bundle at the office.
Thanks,
Matt
^ permalink raw reply
* Re: [PATCH] git-revert is one of the most misunderstood command in git, help users out.
From: Robin Rosenberg @ 2007-11-06 21:21 UTC (permalink / raw)
To: Mike Hommey
Cc: Johannes Schindelin, Junio C Hamano, Steven Grimm,
Pierre Habouzit, git
In-Reply-To: <20071106201324.GA30262@glandium.org>
tisdag 06 november 2007 skrev Mike Hommey:
> Maybe the documentation could emphasise on how to undo things when the
> user makes mistakes.
> Sometimes, saving your repo can be as simple as git reset --hard HEAD@{1}.
> This is not, unfortunately, a works-for-all-cases command.
Yea, git-undo(7).
-- robin
^ permalink raw reply
* [PATCH] make display of total transferred fully accurate
From: Nicolas Pitre @ 2007-11-06 21:30 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
The minimum delay of 1/2 sec between successive throughput updates might
not have been elapsed when display_throughput() is called for the last
time, potentially making the display of total transferred bytes not
right when progress is said to be done.
Let's force an update of the throughput display as well when the
progress is complete. As a side effect, the total transferred will
always be displayed even if the actual transfer rate doesn't have time
to kickin.
Signed-off-by: Nicolas Pitre <nico@cam.org>
---
It was seriously bugging me that the total still didn't match the size
of received packs exactly, even after my previous attempt at fixing it.
diff --git a/progress.c b/progress.c
index a963bd8..2a5a2da 100644
--- a/progress.c
+++ b/progress.c
@@ -14,11 +14,12 @@
#define TP_IDX_MAX 8
struct throughput {
+ off_t curr_total;
off_t prev_total;
struct timeval prev_tv;
unsigned int avg_bytes;
- unsigned int last_bytes[TP_IDX_MAX];
unsigned int avg_misecs;
+ unsigned int last_bytes[TP_IDX_MAX];
unsigned int last_misecs[TP_IDX_MAX];
unsigned int idx;
char display[32];
@@ -109,6 +110,30 @@ static int display(struct progress *progress, unsigned n, int done)
return 0;
}
+static void throughput_string(struct throughput *tp, off_t total,
+ unsigned int rate)
+{
+ int l = sizeof(tp->display);
+ if (total > 1 << 30) {
+ l -= snprintf(tp->display, l, ", %u.%2.2u GiB",
+ (int)(total >> 30),
+ (int)(total & ((1 << 30) - 1)) / 10737419);
+ } else if (total > 1 << 20) {
+ l -= snprintf(tp->display, l, ", %u.%2.2u MiB",
+ (int)(total >> 20),
+ ((int)(total & ((1 << 20) - 1)) * 100) >> 20);
+ } else if (total > 1 << 10) {
+ l -= snprintf(tp->display, l, ", %u.%2.2u KiB",
+ (int)(total >> 10),
+ ((int)(total & ((1 << 10) - 1)) * 100) >> 10);
+ } else {
+ l -= snprintf(tp->display, l, ", %u bytes", (int)total);
+ }
+ if (rate)
+ snprintf(tp->display + sizeof(tp->display) - l, l,
+ " | %u KiB/s", rate);
+}
+
void display_throughput(struct progress *progress, off_t total)
{
struct throughput *tp;
@@ -124,11 +149,12 @@ void display_throughput(struct progress *progress, off_t total)
if (!tp) {
progress->throughput = tp = calloc(1, sizeof(*tp));
if (tp) {
- tp->prev_total = total;
+ tp->prev_total = tp->curr_total = total;
tp->prev_tv = tv;
}
return;
}
+ tp->curr_total = total;
/*
* We have x = bytes and y = microsecs. We want z = KiB/s:
@@ -149,39 +175,21 @@ void display_throughput(struct progress *progress, off_t total)
misecs += (int)(tv.tv_usec - tp->prev_tv.tv_usec) / 977;
if (misecs > 512) {
- int l = sizeof(tp->display);
- unsigned int count = total - tp->prev_total;
+ unsigned int count, rate;
+
+ count = total - tp->prev_total;
tp->prev_total = total;
tp->prev_tv = tv;
tp->avg_bytes += count;
tp->avg_misecs += misecs;
-
- if (total > 1 << 30) {
- l -= snprintf(tp->display, l, ", %u.%2.2u GiB",
- (int)(total >> 30),
- (int)(total & ((1 << 30) - 1)) / 10737419);
- } else if (total > 1 << 20) {
- l -= snprintf(tp->display, l, ", %u.%2.2u MiB",
- (int)(total >> 20),
- ((int)(total & ((1 << 20) - 1))
- * 100) >> 20);
- } else if (total > 1 << 10) {
- l -= snprintf(tp->display, l, ", %u.%2.2u KiB",
- (int)(total >> 10),
- ((int)(total & ((1 << 10) - 1))
- * 100) >> 10);
- } else {
- l -= snprintf(tp->display, l, ", %u bytes", (int)total);
- }
- snprintf(tp->display + sizeof(tp->display) - l, l,
- " | %u KiB/s", tp->avg_bytes / tp->avg_misecs);
-
+ rate = tp->avg_bytes / tp->avg_misecs;
tp->avg_bytes -= tp->last_bytes[tp->idx];
tp->avg_misecs -= tp->last_misecs[tp->idx];
tp->last_bytes[tp->idx] = count;
tp->last_misecs[tp->idx] = misecs;
tp->idx = (tp->idx + 1) % TP_IDX_MAX;
+ throughput_string(tp, total, rate);
if (progress->last_value != -1 && progress_update)
display(progress, progress->last_value, 0);
}
@@ -225,6 +233,12 @@ void stop_progress(struct progress **p_progress)
*p_progress = NULL;
if (progress->last_value != -1) {
/* Force the last update */
+ struct throughput *tp = progress->throughput;
+ if (tp) {
+ unsigned int rate = !tp->avg_misecs ? 0 :
+ tp->avg_bytes / tp->avg_misecs;
+ throughput_string(tp, tp->curr_total, rate);
+ }
progress_update = 1;
display(progress, progress->last_value, 1);
}
^ permalink raw reply related
* Re: [PATCH] git-cvsimport: Add -N option to force a new import
From: Piet Delaney @ 2007-11-06 21:35 UTC (permalink / raw)
To: git; +Cc: piet delaney
In-Reply-To: <1194320170.21645.72.camel@localhost>
Matt McCutchen wrote:
> On Wed, 2007-10-24 at 20:17 -0700, Junio C Hamano wrote:
>> Matt McCutchen <matt@mattmccutchen.net> writes:
>>
>>> I had a git repository for development of rsync and wanted to start
>>> importing the upstream CVS with git-cvsimport, but git-cvsimport saw
>>> that the git repository existed and insisted on updating a previous
>>> import. This patch adds an -N option to git-cvsimport to force a new
>>> import and updates the documentation appropriately.
>> Sounds like a useful addition. Tests?
>
> Good call! As I was dismayed to discover, the existing files in the
> working tree confused the import,\
What's the confusion? I thought updating of the git repo from cvs
worked. Sigh.
-piet
> so -N as I implemented it is useless.
> I ended up doing the import a different way; I'll notify the list if I
> get around to implementing -N properly.
>
> Matt
>
^ permalink raw reply
* Re: [PATCH] Documentation: enhanced "git for CVS users" doc about shared repositories
From: Francesco Pretto @ 2007-11-06 21:44 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v8x5cmern.fsf@gitster.siamese.dyndns.org>
Signed-off-by: Francesco Pretto <ceztkoml@gmail.com>
---
More detailed instructions on how to set up shared repositories.
Removed an old reference to the need of setting umask of ssh
users of shared repositories.
Added a reference to "git for CVS users" doc in git-init manual.
Documentation/cvs-migration.txt | 61 +++++++++++++++++++++++++++++++++++----
Documentation/git-init.txt | 7 ++++
2 files changed, 62 insertions(+), 6 deletions(-)
diff --git a/Documentation/cvs-migration.txt b/Documentation/cvs-migration.txt
index 3b6b494..849b403 100644
--- a/Documentation/cvs-migration.txt
+++ b/Documentation/cvs-migration.txt
@@ -71,7 +71,40 @@ Setting Up a Shared Repository
We assume you have already created a git repository for your project,
possibly created from scratch or from a tarball (see the
link:tutorial.html[tutorial]), or imported from an already existing CVS
-repository (see the next section).
+repository (see the next section). Moreover, we assume you can write in a
+public accessible directory and give other users the permission to do so.
+You could need or not admin privileges to do so, depending on your
+system configuration and how you decide to export the repository.
+
+It's recommended, but not strictly necessary, to create a specific group for
+every project/repository you'll want to create, so it will be easier to give
+or prevent access of users to specific repositories. With admin privilege launch:
+
+------------------------------------------------
+$ groupadd $group
+------------------------------------------------
+
+If you want to add an user to this group, launch:
+
+------------------------------------------------
+$ usermod -a -G $group $username
+------------------------------------------------
+
+In our example, we will store the shared repository in the /pub dir, so the
+user creating it will need write permission there. There's no problems if you
+choose another directory, but you'll have to ensure it will be accessible by
+other users, on local or by remote (this could be not the case of home
+directories).
+
+If you just want to create a directory that is writable by every users that have
+a local account, launch with privileged credentials:
+
+------------------------------------------------
+$ mkdir /pub
+$ chmod a+w,+t /pub
+------------------------------------------------
+
+Now you can proceed with an unprivileged user.
Assume your existing repo is at /home/alice/myproject. Create a new "bare"
repository (a repository without a working tree) and fetch your project into
@@ -84,21 +117,37 @@ $ git --bare init --shared
$ git --bare fetch /home/alice/myproject master:master
------------------------------------------------
+If you previously decided to create a specific group for the committers of the
+repository, assign its ownership to that group (you'll have to be a member of it
+or switch to privileged credentials):
+
+------------------------------------------------
+$ chgrp -R $group /pub/my-repo.git
+------------------------------------------------
+
Next, give every team member read/write access to this repository. One
easy way to do this is to give all the team members ssh access to the
machine where the repository is hosted. If you don't want to give them a
full shell on the machine, there is a restricted shell which only allows
users to do git pushes and pulls; see gitlink:git-shell[1].
-Put all the committers in the same group, and make the repository
-writable by that group:
+The following two commands will require admin privileges; first, enable
+git-shell putting it on the trusted shells list of the system:
------------------------------------------------
-$ chgrp -R $group /pub/my-repo.git
+$ echo `which git-shell` >> /etc/shells
+------------------------------------------------
+
+Now, let's create the commit users:
+
+------------------------------------------------
+$ useradd -g $group -s `which git-shell` $username
------------------------------------------------
-Make sure committers have a umask of at most 027, so that the directories
-they create are writable and searchable by other group members.
+These users will be enabled to push on repositories owned by the group $group.
+Later, you can give access to other projects simply by adding them to
+other groups. Similarly, you can prevent access to repositories simply
+removing those users from related groups.
Importing a CVS archive
-----------------------
diff --git a/Documentation/git-init.txt b/Documentation/git-init.txt
index 07484a4..f5f363d 100644
--- a/Documentation/git-init.txt
+++ b/Documentation/git-init.txt
@@ -101,6 +101,13 @@ $ git-add . <2>
<2> add all existing file to the index
+SHARED REPOSITORIES
+-------------------
+
+Please refer to link:cvs-migration.html[git for CVS users], section "Setting Up
+a Shared Repository", for details on how to set up shared repositories.
+
+
Author
------
Written by Linus Torvalds <torvalds@osdl.org>
^ permalink raw reply related
* Re: git-bundle questions
From: Jakub Narebski @ 2007-11-06 21:49 UTC (permalink / raw)
To: git
In-Reply-To: <1194382443.5568.13.camel@localhost.localdomain>
Matthew Booth wrote:
> Secondly, has anybody given any thought to allowing a user to create a
> bundle from any of the web front-ends? The reason for this request is
> that I've been doing some work for a company where only corporate
> standard desktops have internet access. They run Windows, and you can't
> install additional software on them. This makes interacting with an
> external git repository from my development servers problematic. Git
> bundles do at least make this possible, but I have to create a bundle on
> my laptop when I'm at home and transfer it to a server when I get to the
> office. This would be a whole lot easier if I could just download the
> bundle at the office.
I have though about adding such support to gitweb. The problem lies
in how to specify the commits and refs to bundle.
Perhaps separate CGI script would be an answer.
--
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
^ permalink raw reply
* Re: [PATCH] git-mailsplit: with maildirs try to process new/ if cur/ is empty
From: Alex Riesen @ 2007-11-06 21:50 UTC (permalink / raw)
To: Karl Hasselström
Cc: Johannes Schindelin, Jeff King, Michael Cohen, Gerrit Pape,
Fernando J. Pereda, git, Junio C Hamano
In-Reply-To: <20071106163548.GA8207@diana.vm.bytemark.co.uk>
Karl Hasselström, Tue, Nov 06, 2007 17:35:48 +0100:
> On 2007-11-06 15:51:09 +0000, Johannes Schindelin wrote:
> > On Tue, 6 Nov 2007, Jeff King wrote:
> > > On Tue, Nov 06, 2007 at 11:01:03AM +0000, Johannes Schindelin wrote:
> > > > I fail to see how the absence of one of cur/ or new/ can lead to
> > > > the absence of patches. You could forget to save some patches,
> > > > yes, but the presence of cur/ and new/ is no indicator for that.
> > >
> > > Read my message again. Alex is proposing ignoring errors in
> > > opening the directories; I am proposing ignoring such errors
> > > _only_ when the error is that the directory does not exist.
> > >
> > > IOW, if there is some other error in opening the directory, it
> > > should be fatal, because you might be missing patches.
> >
> > Yeah, sorry, I missed that.
>
> I think it might actually not be totally unreasonable to error out
> unless both directories exist. From
> http://www.qmail.org/qmail-manual-html/man5/maildir.html:
>
> A directory in maildir format has three subdirectories, all on the
> same filesystem: tmp, new, and cur.
>
> In other words, if it doesn't have these three directories, it isn't a
> Maildir directory.
On the same line of reasoning, if opening a ".../cur" fails with ENOTDIR,
it must be not a Maildir...
> On the other hand, one could argue that requiring both dirs to exist
> is being too picky.
...which MUST NOT mean it does not contain useful patches.
IOW, the tool can try and apply everything it finds.
If user told it to get patches from the...whatever, then the patches
should it get and damn qmail.
^ permalink raw reply
* git-rev-list diff options broken
From: Han-Wen Nienhuys @ 2007-11-06 22:14 UTC (permalink / raw)
To: git
Hi,
the git-rev-list manpage says
**
Diff Formatting
~~~~~~~~~~~~~~~
Below are listed options that control the formatting of diff output.
Some of them are specific to gitlink:git-rev-list[1], however other diff
options may be given. See gitlink:git-diff-files[1] for more options.
**
however, the source code says
if ((!list &&
(!(revs.tag_objects||revs.tree_objects||revs.blob_objects) &&
!revs.pending.nr)) ||
revs.diff)
usage(rev_list_usage);
so any attempt at showing diffs with git-rev-list will fail. What's
the deal with this?
--
Han-Wen Nienhuys - hanwen@xs4all.nl - http://www.xs4all.nl/~hanwen
^ permalink raw reply
* Re: git-rev-list diff options broken
From: Han-Wen Nienhuys @ 2007-11-06 22:15 UTC (permalink / raw)
To: git
In-Reply-To: <f329bf540711061414k1627521bvaf4a7a06460cc3fd@mail.gmail.com>
2007/11/6, Han-Wen Nienhuys <hanwenn@gmail.com>:
> so any attempt at showing diffs with git-rev-list will fail. What's
> the deal with this?
this is with
git version 1.5.3.5.576.gfe6193
--
Han-Wen Nienhuys - hanwen@xs4all.nl - http://www.xs4all.nl/~hanwen
^ permalink raw reply
* Re: [PATCH] git-revert is one of the most misunderstood command in git, help users out.
From: Johannes Schindelin @ 2007-11-06 22:21 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Pierre Habouzit, Steven Grimm, git
In-Reply-To: <7vir4fgnz1.fsf@gitster.siamese.dyndns.org>
Hi,
On Tue, 6 Nov 2007, Junio C Hamano wrote:
> Pierre Habouzit <madcoder@debian.org> writes:
>
> > On Tue, Nov 06, 2007 at 06:27:54PM +0000, Johannes Schindelin wrote:
> >
> >> On Tue, 6 Nov 2007, Junio C Hamano wrote:
> >>
> >> > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> >> >
> >> > > Well, I think that _if_ we allow "git revert <path>" to mean
> >> > > "revert the changes to <path>, relative to the index" (which
> >> > > would be the same as "git checkout <path>"), then committing that
> >> > > change just does not make sense.
> >> > >
> >> > > And it is this behaviour that people are seeking, not "git revert
> >> > > <commit> <path>".
> >> >
> >> > Heh, I found this in the recent log somewhere.
> >> >
> >> > <gitte> Really, I wonder how difficult git is for people who are not
> >> > brainwashed by cvs/svn, and unfortunately enough, partly by bzr
> >> > and hg.
> >> > <gitte> From a user perspective, you might be correct. But then we
> >> > have to add 1000 commands to reflect the English language.
> >> > <gitte> Not what I want. [06:46]
> >> >
> >> > I am wondering who said it ;-).
> >>
> >> Now, that is not fair, using my own words against me ;-)
> >
> > That's very funny actually :]
>
> Yeah, it was doubly funny after I saw you posted a list of "$scm revert"
> and Dscho still sided with you in that thread.
Hey, I had my nice 5 minutes for the day, so give me a break!
;-)
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] git-revert is one of the most misunderstood command in git, help users out.
From: Johannes Schindelin @ 2007-11-06 22:25 UTC (permalink / raw)
To: Robin Rosenberg
Cc: Mike Hommey, Junio C Hamano, Steven Grimm, Pierre Habouzit, git
In-Reply-To: <200711062221.58475.robin.rosenberg.lists@dewire.com>
Hi,
On Tue, 6 Nov 2007, Robin Rosenberg wrote:
> tisdag 06 november 2007 skrev Mike Hommey:
> > Maybe the documentation could emphasise on how to undo things when the
> > user makes mistakes.
> > Sometimes, saving your repo can be as simple as git reset --hard HEAD@{1}.
> > This is not, unfortunately, a works-for-all-cases command.
>
> Yea, git-undo(7).
In related news, I know a few users who need an un-rm-rf. Anyone?
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 3/3] pretty=format: Avoid some expensive calculations when not needed
From: René Scharfe @ 2007-11-06 22:31 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, git
In-Reply-To: <7vejf4kwry.fsf@gitster.siamese.dyndns.org>
Junio C Hamano schrieb:
> René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
>
>> Junio C Hamano schrieb: ...
>>> Instead of allocating a separate array and freeing at the end,
>>> wouldn't it make more sense to have a bitfield that records what
>>> is used by the format string inside the array elements?
>> How about (ab)using the value field? Let interp_find_active() mark
>> unneeded entries with NULL, and the rest with some cookie. All
>> table entries with non-NULL values need to be initialized.
>> interp_set_entry() needs to be aware of this cookie, as it mustn't
>> free() it. The cookie could be the address of a static char* in
>> interpolate.c.
>
> Sorry, where is this aversion to making the struct a bit larger
> coming from?
Not from the rational part of my brain, for sure. The following on
top of Dscho's second patch? (A char would be smaller, but a bitfield
documents the intent better.)
diff --git a/interpolate.c b/interpolate.c
index 80eeb36..1e4ccaf 100644
--- a/interpolate.c
+++ b/interpolate.c
@@ -103,22 +103,21 @@ unsigned long interpolate(char *result, unsigned long reslen,
return newlen;
}
-char *interp_find_active(const char *orig,
- const struct interp *interps, int ninterps)
+void interp_find_active(const char *orig, struct interp *interps, int ninterps)
{
- char *result = xcalloc(1, ninterps);
char c;
int i;
+ for (i = 0; i < ninterps; i++)
+ interps[i].active = 0;
+
while ((c = *(orig++)))
if (c == '%')
/* Try to match an interpolation string. */
for (i = 0; i < ninterps; i++)
if (!prefixcmp(orig, interps[i].name + 1)) {
- result[i] = 1;
+ interps[i].active = 1;
orig += strlen(interps[i].name + 1);
break;
}
-
- return result;
}
diff --git a/interpolate.h b/interpolate.h
index 2d197c5..a8ee6b9 100644
--- a/interpolate.h
+++ b/interpolate.h
@@ -14,6 +14,7 @@
struct interp {
const char *name;
char *value;
+ unsigned active:1;
};
extern void interp_set_entry(struct interp *table, int slot, const char *value);
@@ -22,7 +23,6 @@ extern void interp_clear_table(struct interp *table, int ninterps);
extern unsigned long interpolate(char *result, unsigned long reslen,
const char *orig,
const struct interp *interps, int ninterps);
-extern char *interp_find_active(const char *orig,
- const struct interp *interps, int ninterps);
+extern void interp_find_active(const char *orig, struct interp *interps, int ninterps);
#endif /* INTERPOLATE_H */
^ permalink raw reply related
* Re: [PATCH] Give git-am back the ability to add Signed-off-by lines.
From: Pierre Habouzit @ 2007-11-06 22:35 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Junio C Hamano, git
In-Reply-To: <200711062133.58903.johannes.sixt@telecom.at>
[-- Attachment #1: Type: text/plain, Size: 888 bytes --]
On Tue, Nov 06, 2007 at 08:33:58PM +0000, Johannes Sixt wrote:
> This was lost in the migration to git-rev-parse --parseopt by commit
> 78443d90491c1b82afdffc3d5d2ab8c1a58928b5.
>
> Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
> ---
> git-am.sh | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/git-am.sh b/git-am.sh
> index 876b973..e5af955 100755
> --- a/git-am.sh
> +++ b/git-am.sh
> @@ -131,7 +131,7 @@ do
> binary=t ;;
> -3|--3way)
> threeway=t ;;
> - -s--signoff)
> + -s|--signoff)
Gaaaah. One more case of t/f finger-slip
> sign=t ;;
> -u|--utf8)
> utf8=t ;; # this is now default
> --
> 1.5.3.4.315.g2ce38
>
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* git-log -p --raw broken?
From: Han-Wen Nienhuys @ 2007-11-06 22:48 UTC (permalink / raw)
To: git
Hi,
I'm trying to get a list of
git diff-tree -t -r --raw COMMITTISH
where COMMITTISH comes from git-rev-list, using a limited number of
processes. The manual page suggests that I should be able to do
git log -p --raw COMMIT-RANGE
however, when I try that, I get always get the same style output,
which is the (to me: useless) human readable output.
Am I missing something?
--
Han-Wen Nienhuys - hanwen@xs4all.nl - http://www.xs4all.nl/~hanwen
^ permalink raw reply
* Re: git-log -p --raw broken?
From: Han-Wen Nienhuys @ 2007-11-06 23:03 UTC (permalink / raw)
To: git
In-Reply-To: <f329bf540711061448iab9d4a9q37e13b846dbc5ff1@mail.gmail.com>
2007/11/6, Han-Wen Nienhuys <hanwenn@gmail.com>:
> I'm trying to get a list of
>
> git diff-tree -t -r --raw COMMITTISH
>
> [..]
> Am I missing something?
I probably am, never mind this message.
\
--
Han-Wen Nienhuys - hanwen@xs4all.nl - http://www.xs4all.nl/~hanwen
^ permalink raw reply
* Re: git-log -p --raw broken?
From: Linus Torvalds @ 2007-11-06 23:10 UTC (permalink / raw)
To: hanwen; +Cc: git
In-Reply-To: <f329bf540711061448iab9d4a9q37e13b846dbc5ff1@mail.gmail.com>
On Tue, 6 Nov 2007, Han-Wen Nienhuys wrote:
>
> The manual page suggests that I should be able to do
>
> git log -p --raw COMMIT-RANGE
>
> however, when I try that, I get always get the same style output,
> which is the (to me: useless) human readable output.
>
> Am I missing something?
No, you're not *missing* anything. You have something *too much*.
Please remove the "-p" if you don't want a patch.
This works:
git log --raw COMMIT-RANGE
(actually, most of the time you'd want to use "-r" too when you get raw
output, but git log enables that by default)
Linus
^ permalink raw reply
* Re: [PATCH 3/3] pretty=format: Avoid some expensive calculations when not needed
From: René Scharfe @ 2007-11-06 23:17 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, git
In-Reply-To: <4730EB4E.4080903@lsrfire.ath.cx>
By the way, the more intrusive surgery required when using strbuf_expand()
leads to even faster operation. Here my measurements of most of Paul's
test cases (best of three runs):
# master
$ time git log --pretty=oneline >/dev/null
real 0m0.390s
user 0m0.340s
sys 0m0.040s
# master
$ time git log --pretty=raw >/dev/null
real 0m0.434s
user 0m0.408s
sys 0m0.016s
# master
$ time git log --pretty="format:%H {%P} %ct" >/dev/null
real 0m1.347s
user 0m0.080s
sys 0m1.256s
# interp_find_active
$ time ./git log --pretty="format:%H {%P} %ct" >/dev/null
real 0m0.694s
user 0m0.020s
sys 0m0.672s
# strbuf_expand
$ time ./git log --pretty="format:%H {%P} %ct" >/dev/null
real 0m0.395s
user 0m0.352s
sys 0m0.028s
I haven't seen any comments on strbuf_expand. Is it too far out?
Here it is again, adjusted for current master and with the changes
to strbuf.[ch] coming first:
strbuf.c | 22 +++++
strbuf.h | 3
pretty.c | 276 ++++++++++++++++++++++++++++++++++-----------------------------
3 files changed, 178 insertions(+), 123 deletions(-)
diff --git a/strbuf.c b/strbuf.c
index f4201e1..b71da99 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -129,6 +129,28 @@ void strbuf_addf(struct strbuf *sb, const char *fmt, ...)
strbuf_setlen(sb, sb->len + len);
}
+void strbuf_expand(struct strbuf *sb, const char *fmt,
+ const char **placeholders, expand_fn_t fn, void *context)
+{
+ char c;
+ const char **p;
+
+ while ((c = *fmt++)) {
+ if (c != '%') {
+ strbuf_addch(sb, c);
+ continue;
+ }
+
+ for (p = placeholders; *p; p++) {
+ if (!prefixcmp(fmt, *p)) {
+ fn(sb, *p, context);
+ fmt += strlen(*p);
+ break;
+ }
+ }
+ }
+}
+
size_t strbuf_fread(struct strbuf *sb, size_t size, FILE *f)
{
size_t res;
diff --git a/strbuf.h b/strbuf.h
index cd7f295..95071d5 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -102,6 +102,9 @@ static inline void strbuf_addbuf(struct strbuf *sb, struct strbuf *sb2) {
strbuf_add(sb, sb2->buf, sb2->len);
}
+typedef void (*expand_fn_t) (struct strbuf *sb, const char *placeholder, void *context);
+extern void strbuf_expand(struct strbuf *sb, const char *fmt, const char **placeholders, expand_fn_t fn, void *context);
+
__attribute__((format(printf,2,3)))
extern void strbuf_addf(struct strbuf *sb, const char *fmt, ...);
diff --git a/pretty.c b/pretty.c
index 490cede..ea644bb 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1,6 +1,5 @@
#include "cache.h"
#include "commit.h"
-#include "interpolate.h"
#include "utf8.h"
#include "diff.h"
#include "revision.h"
@@ -283,7 +282,8 @@ static char *logmsg_reencode(const struct commit *commit,
return out;
}
-static void fill_person(struct interp *table, const char *msg, int len)
+static void format_person_part(struct strbuf *sb, char part,
+ const char *msg, int len)
{
int start, end, tz = 0;
unsigned long date;
@@ -295,7 +295,10 @@ static void fill_person(struct interp *table, const char *msg, int len)
start = end + 1;
while (end > 0 && isspace(msg[end - 1]))
end--;
- table[0].value = xmemdupz(msg, end);
+ if (part == 'n') { /* name */
+ strbuf_add(sb, msg, end);
+ return;
+ }
if (start >= len)
return;
@@ -307,7 +310,10 @@ static void fill_person(struct interp *table, const char *msg, int len)
if (end >= len)
return;
- table[1].value = xmemdupz(msg + start, end - start);
+ if (part == 'e') { /* email */
+ strbuf_add(sb, msg + start, end - start);
+ return;
+ }
/* parse date */
for (start = end + 1; start < len && isspace(msg[start]); start++)
@@ -318,7 +324,10 @@ static void fill_person(struct interp *table, const char *msg, int len)
if (msg + start == ep)
return;
- table[5].value = xmemdupz(msg + start, ep - (msg + start));
+ if (part == 't') { /* date, UNIX timestamp */
+ strbuf_add(sb, msg + start, ep - (msg + start));
+ return;
+ }
/* parse tz */
for (start = ep - msg + 1; start < len && isspace(msg[start]); start++)
@@ -329,123 +338,107 @@ static void fill_person(struct interp *table, const char *msg, int len)
tz = -tz;
}
- interp_set_entry(table, 2, show_date(date, tz, DATE_NORMAL));
- interp_set_entry(table, 3, show_date(date, tz, DATE_RFC2822));
- interp_set_entry(table, 4, show_date(date, tz, DATE_RELATIVE));
- interp_set_entry(table, 6, show_date(date, tz, DATE_ISO8601));
+ switch (part) {
+ case 'd': /* date */
+ strbuf_addstr(sb, show_date(date, tz, DATE_NORMAL));
+ return;
+ case 'D': /* date, RFC2822 style */
+ strbuf_addstr(sb, show_date(date, tz, DATE_RFC2822));
+ return;
+ case 'r': /* date, relative */
+ strbuf_addstr(sb, show_date(date, tz, DATE_RELATIVE));
+ return;
+ case 'i': /* date, ISO 8601 */
+ strbuf_addstr(sb, show_date(date, tz, DATE_ISO8601));
+ return;
+ }
}
-void format_commit_message(const struct commit *commit,
- const void *format, struct strbuf *sb)
+static void format_commit_item(struct strbuf *sb, const char *placeholder,
+ void *context)
{
- struct interp table[] = {
- { "%H" }, /* commit hash */
- { "%h" }, /* abbreviated commit hash */
- { "%T" }, /* tree hash */
- { "%t" }, /* abbreviated tree hash */
- { "%P" }, /* parent hashes */
- { "%p" }, /* abbreviated parent hashes */
- { "%an" }, /* author name */
- { "%ae" }, /* author email */
- { "%ad" }, /* author date */
- { "%aD" }, /* author date, RFC2822 style */
- { "%ar" }, /* author date, relative */
- { "%at" }, /* author date, UNIX timestamp */
- { "%ai" }, /* author date, ISO 8601 */
- { "%cn" }, /* committer name */
- { "%ce" }, /* committer email */
- { "%cd" }, /* committer date */
- { "%cD" }, /* committer date, RFC2822 style */
- { "%cr" }, /* committer date, relative */
- { "%ct" }, /* committer date, UNIX timestamp */
- { "%ci" }, /* committer date, ISO 8601 */
- { "%e" }, /* encoding */
- { "%s" }, /* subject */
- { "%b" }, /* body */
- { "%Cred" }, /* red */
- { "%Cgreen" }, /* green */
- { "%Cblue" }, /* blue */
- { "%Creset" }, /* reset color */
- { "%n" }, /* newline */
- { "%m" }, /* left/right/bottom */
- };
- enum interp_index {
- IHASH = 0, IHASH_ABBREV,
- ITREE, ITREE_ABBREV,
- IPARENTS, IPARENTS_ABBREV,
- IAUTHOR_NAME, IAUTHOR_EMAIL,
- IAUTHOR_DATE, IAUTHOR_DATE_RFC2822, IAUTHOR_DATE_RELATIVE,
- IAUTHOR_TIMESTAMP, IAUTHOR_ISO8601,
- ICOMMITTER_NAME, ICOMMITTER_EMAIL,
- ICOMMITTER_DATE, ICOMMITTER_DATE_RFC2822,
- ICOMMITTER_DATE_RELATIVE, ICOMMITTER_TIMESTAMP,
- ICOMMITTER_ISO8601,
- IENCODING,
- ISUBJECT,
- IBODY,
- IRED, IGREEN, IBLUE, IRESET_COLOR,
- INEWLINE,
- ILEFT_RIGHT,
- };
+ const struct commit *commit = context;
struct commit_list *p;
- char parents[1024];
- unsigned long len;
int i;
enum { HEADER, SUBJECT, BODY } state;
const char *msg = commit->buffer;
- if (ILEFT_RIGHT + 1 != ARRAY_SIZE(table))
- die("invalid interp table!");
-
/* these are independent of the commit */
- interp_set_entry(table, IRED, "\033[31m");
- interp_set_entry(table, IGREEN, "\033[32m");
- interp_set_entry(table, IBLUE, "\033[34m");
- interp_set_entry(table, IRESET_COLOR, "\033[m");
- interp_set_entry(table, INEWLINE, "\n");
+ switch (placeholder[0]) {
+ case 'C':
+ switch (placeholder[3]) {
+ case 'd': /* red */
+ strbuf_addstr(sb, "\033[31m");
+ return;
+ case 'e': /* green */
+ strbuf_addstr(sb, "\033[32m");
+ return;
+ case 'u': /* blue */
+ strbuf_addstr(sb, "\033[34m");
+ return;
+ case 's': /* reset color */
+ strbuf_addstr(sb, "\033[m");
+ return;
+ }
+ case 'n': /* newline */
+ strbuf_addch(sb, '\n');
+ return;
+ }
/* these depend on the commit */
if (!commit->object.parsed)
parse_object(commit->object.sha1);
- interp_set_entry(table, IHASH, sha1_to_hex(commit->object.sha1));
- interp_set_entry(table, IHASH_ABBREV,
- find_unique_abbrev(commit->object.sha1,
- DEFAULT_ABBREV));
- interp_set_entry(table, ITREE, sha1_to_hex(commit->tree->object.sha1));
- interp_set_entry(table, ITREE_ABBREV,
- find_unique_abbrev(commit->tree->object.sha1,
- DEFAULT_ABBREV));
- interp_set_entry(table, ILEFT_RIGHT,
- (commit->object.flags & BOUNDARY)
- ? "-"
- : (commit->object.flags & SYMMETRIC_LEFT)
- ? "<"
- : ">");
-
- 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);
+ switch (placeholder[0]) {
+ case 'H': /* commit hash */
+ strbuf_addstr(sb, sha1_to_hex(commit->object.sha1));
+ return;
+ case 'h': /* abbreviated commit hash */
+ strbuf_addstr(sb, find_unique_abbrev(commit->object.sha1,
+ DEFAULT_ABBREV));
+ return;
+ case 'T': /* tree hash */
+ strbuf_addstr(sb, sha1_to_hex(commit->tree->object.sha1));
+ return;
+ case 't': /* abbreviated tree hash */
+ strbuf_addstr(sb, find_unique_abbrev(commit->tree->object.sha1,
+ DEFAULT_ABBREV));
+ return;
+ case 'P': /* parent hashes */
+ for (p = commit->parents; p; p = p->next) {
+ if (p != commit->parents)
+ strbuf_addch(sb, ' ');
+ strbuf_addstr(sb, sha1_to_hex(p->item->object.sha1));
+ }
+ return;
+ case 'p': /* abbreviated parent hashes */
+ for (p = commit->parents; p; p = p->next) {
+ if (p != commit->parents)
+ strbuf_addch(sb, ' ');
+ strbuf_addstr(sb, find_unique_abbrev(
+ p->item->object.sha1, DEFAULT_ABBREV));
+ }
+ return;
+ case 'm': /* left/right/bottom */
+ strbuf_addch(sb, (commit->object.flags & BOUNDARY)
+ ? '-'
+ : (commit->object.flags & SYMMETRIC_LEFT)
+ ? '<'
+ : '>');
+ return;
+ }
+
+ /* For the rest we have to parse the commit header. */
for (i = 0, state = HEADER; msg[i] && state < BODY; i++) {
int eol;
for (eol = i; msg[eol] && msg[eol] != '\n'; eol++)
; /* do nothing */
if (state == SUBJECT) {
- table[ISUBJECT].value = xmemdupz(msg + i, eol - i);
+ if (placeholder[0] == 's') {
+ strbuf_add(sb, msg + i, eol - i);
+ return;
+ }
i = eol;
}
if (i == eol) {
@@ -453,29 +446,66 @@ void format_commit_message(const struct commit *commit,
/* strip empty lines */
while (msg[eol + 1] == '\n')
eol++;
- } else if (!prefixcmp(msg + i, "author "))
- fill_person(table + IAUTHOR_NAME,
- msg + i + 7, eol - i - 7);
- else if (!prefixcmp(msg + i, "committer "))
- fill_person(table + ICOMMITTER_NAME,
- msg + i + 10, eol - i - 10);
- else if (!prefixcmp(msg + i, "encoding "))
- table[IENCODING].value =
- xmemdupz(msg + i + 9, eol - i - 9);
+ } else if (!prefixcmp(msg + i, "author ")) {
+ if (placeholder[0] == 'a') {
+ format_person_part(sb, placeholder[1],
+ msg + i + 7, eol - i - 7);
+ return;
+ }
+ } else if (!prefixcmp(msg + i, "committer ")) {
+ if (placeholder[0] == 'c') {
+ format_person_part(sb, placeholder[1],
+ msg + i + 10, eol - i - 10);
+ return;
+ }
+ } else if (!prefixcmp(msg + i, "encoding ")) {
+ if (placeholder[0] == 'e') {
+ strbuf_add(sb, msg + i + 9, eol - i - 9);
+ return;
+ }
+ }
i = eol;
}
- if (msg[i])
- table[IBODY].value = xstrdup(msg + i);
-
- len = interpolate(sb->buf + sb->len, strbuf_avail(sb),
- format, table, ARRAY_SIZE(table));
- if (len > strbuf_avail(sb)) {
- strbuf_grow(sb, len);
- interpolate(sb->buf + sb->len, strbuf_avail(sb) + 1,
- format, table, ARRAY_SIZE(table));
- }
- strbuf_setlen(sb, sb->len + len);
- interp_clear_table(table, ARRAY_SIZE(table));
+ if (msg[i] && placeholder[0] == 'b') /* body */
+ strbuf_addstr(sb, msg + i);
+}
+
+void format_commit_message(const struct commit *commit,
+ const void *format, struct strbuf *sb)
+{
+ const char *placeholders[] = {
+ "H", /* commit hash */
+ "h", /* abbreviated commit hash */
+ "T", /* tree hash */
+ "t", /* abbreviated tree hash */
+ "P", /* parent hashes */
+ "p", /* abbreviated parent hashes */
+ "an", /* author name */
+ "ae", /* author email */
+ "ad", /* author date */
+ "aD", /* author date, RFC2822 style */
+ "ar", /* author date, relative */
+ "at", /* author date, UNIX timestamp */
+ "ai", /* author date, ISO 8601 */
+ "cn", /* committer name */
+ "ce", /* committer email */
+ "cd", /* committer date */
+ "cD", /* committer date, RFC2822 style */
+ "cr", /* committer date, relative */
+ "ct", /* committer date, UNIX timestamp */
+ "ci", /* committer date, ISO 8601 */
+ "e", /* encoding */
+ "s", /* subject */
+ "b", /* body */
+ "Cred", /* red */
+ "Cgreen", /* green */
+ "Cblue", /* blue */
+ "Creset", /* reset color */
+ "n", /* newline */
+ "m", /* left/right/bottom */
+ NULL
+ };
+ strbuf_expand(sb, format, placeholders, format_commit_item, (void *)commit);
}
static void pp_header(enum cmit_fmt fmt,
^ permalink raw reply related
* [PATCH] Add Documentation/CodingStyle
From: Johannes Schindelin @ 2007-11-06 23:17 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ralf Wildenhues, git
In-Reply-To: <7vtznzf5jb.fsf@gitster.siamese.dyndns.org>
Even if our code is quite a good documentation for our coding style,
some people seem to prefer a document describing it.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
I long resisted in adding this, as I really believe our code
base is a very clean one, and a good description of what we
prefer.
But it seems that not everybody has the time to study our
code in depth, beautiful as it is ;-)
BTW the first to catch the allusion to a certain movie
wins a drink with me.
Documentation/CodingStyle | 87 +++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 87 insertions(+), 0 deletions(-)
create mode 100644 Documentation/CodingStyle
diff --git a/Documentation/CodingStyle b/Documentation/CodingStyle
new file mode 100644
index 0000000..622b80b
--- /dev/null
+++ b/Documentation/CodingStyle
@@ -0,0 +1,87 @@
+As a popular project, we also have some guidelines to keep to the
+code. For git in general, two rough rules are:
+
+ - Most importantly, we never say "It's in POSIX; we'll happily
+ screw your system that does not conform." We live in the
+ real world.
+
+ - However, we often say "Let's stay away from that construct,
+ it's not even in POSIX".
+
+As for more concrete guidelines, just imitate the existing code
+(this is a good guideline, no matter which project you are contributing
+to...). But if you must have some list of rules, here they are.
+
+For shell scripts specifically (not exhaustive):
+
+ - We prefer $( ... ) for command substitution; unlike ``, it
+ properly nests. It should have been the way Bourne spelled
+ it from day one, but unfortunately isn't.
+
+ - We use ${parameter-word} and its [-=?+] siblings, and their
+ colon'ed "unset or null" form.
+
+ - We use ${parameter#word} and its [#%] siblings, and their
+ doubled "longest matching" form.
+
+ - We use Arithmetic Expansion $(( ... )).
+
+ - No "Substring Expansion" ${parameter:offset:length}.
+
+ - No shell arrays.
+
+ - No strlen ${#parameter}.
+
+ - No regexp ${parameter/pattern/string}.
+
+ - We do not use Process Substitution <(list) or >(list).
+
+ - We prefer "test" over "[ ... ]".
+
+ - We do not write noiseword "function" in front of shell
+ functions.
+
+For C programs:
+
+ - Use tabs to increment, and interpret tabs as taking up to 8 spaces
+
+ - Try to keep to 80 characters per line
+
+ - When declaring pointers, the star sides with the variable name, i.e.
+ "char *string", not "char* string" or "char * string". This makes
+ it easier to understand "char *string, c;"
+
+ - Do not use curly brackets unnecessarily. I.e.
+
+ if (bla) {
+ x = 1;
+ }
+
+ is frowned upon. A gray area is when the statement extends over a
+ few lines, and/or you have a lengthy comment atop of it.
+
+ - Try to make your code understandable. You may put comments in, but
+ comments invariably tend to stale out when the code they were
+ describing changes. Often splitting a function into two makes the
+ intention of the code much clearer.
+
+ Double negation is often harder to understand than no negation at
+ all.
+
+ Some clever tricks, like using the !! operator with arithmetic
+ constructs, can be extremely confusing to others. Avoid them,
+ unless there is a compelling reason to use them.
+
+ - Use the API. No, really. We have a strbuf (variable length string),
+ several arrays with the ALLOC_GROW() macro, a path_list for sorted
+ string lists, a hash map (mapping struct objects) named
+ "struct decorate", amongst other things.
+
+ - #include system headers in git-compat-util.h. Some headers on some
+ systems show subtle breakages when you change the order, so it is
+ best to keep them in one place.
+
+ - if you are planning a new command, consider writing it in shell or
+ perl first, so that changes in semantics can be easily changed and
+ discussed. Many git commands started out like that, and a few are
+ still scripts.
--
1.5.3.5.1597.g7191
^ permalink raw reply related
* Re: [PATCH] Documentation: enhanced "git for CVS users" doc about shared repositories
From: Junio C Hamano @ 2007-11-06 23:25 UTC (permalink / raw)
To: Francesco Pretto; +Cc: git
In-Reply-To: <4730E056.7080809@gmail.com>
Francesco Pretto <ceztkoml@gmail.com> writes:
> Signed-off-by: Francesco Pretto <ceztkoml@gmail.com>
> ---
> More detailed instructions on how to set up shared repositories.
> Removed an old reference to the need of setting umask of ssh
> users of shared repositories.
> Added a reference to "git for CVS users" doc in git-init manual.
Honestly speaking, I am not too thrilled about making the
cvs-migration document much longer than what it currently is.
Having said that, let's take a look at each hunk.
> @@ -71,7 +71,40 @@ Setting Up a Shared Repository
> We assume you have already created a git repository for your project,
> possibly created from scratch or from a tarball (see the
> link:tutorial.html[tutorial]), or imported from an already existing CVS
> -repository (see the next section).
> +repository (see the next section). Moreover, we assume you can write in a
> +public accessible directory and give other users the permission to do so.
> +You could need or not admin privileges to do so, depending on your
> +system configuration and how you decide to export the repository.
I do not think the above helps anybody. Later sections say
"make it writable by foo group" and such specifically, and from
such descriptions, the reader either (1) understands what are
prerequisite for being able to do so, or (2) is clueless enough
to get puzzled by failure message from "chgrp git foo", and would
not even make the connection to the above text after seeing such
a failure anyway.
> +It's recommended, but not strictly necessary, to create a specific group for
> +every project/repository you'll want to create, so it will be easier to give
> +or prevent access of users to specific repositories.
I'd say this is not git specific nor cvs migrant specific advice
but falls into a common sense category. Better not clutter the
document with such, and keep it short and readable in one
sitting.
> +... With admin privilege launch:
> +
> +------------------------------------------------
> +$ groupadd $group
> +------------------------------------------------
> +
> +If you want to add an user to this group, launch:
> +
> +------------------------------------------------
> +$ usermod -a -G $group $username
> +------------------------------------------------
I tend to edit /etc/group with vi ;-) and I suspect these two
commands are specific to the distro you happen to use.
For something like "cvs migration", I really do not want a set
of step-by-step hand holding instructions. Just telling them to
"pick a group for the project, make repositories belonging to
the project owned by that group, and make them writable by the
group members" should be enough. CVS migrants may not know how
the world works with respect to git, but they are not idiots.
Introductory UNIX command guide is not the goal of the document.
Try to tell them _what_ needs to happen, not _how_, when that
level of the detail is sufficient.
> +In our example, we will store the shared repository in the /pub dir, so the
> +user creating it will need write permission there. There's no problems if you
> +choose another directory, but you'll have to ensure it will be accessible by
> +other users, on local or by remote (this could be not the case of home
> +directories).
Everything up to "by other users" is good, but ", on local or
by..." are unnecessary. If your stress is on shared
repositories, do not even mention "home", unless you are very
convinced that it is a very typical use case, in which case you
should be certain about what you recommend and there is no place
for expression like "this could be ..." for such a sure
recommendation.
> +If you just want to create a directory that is writable by every users that have
> +a local account, launch with privileged credentials:
> +
> +------------------------------------------------
> +$ mkdir /pub
> +$ chmod a+w,+t /pub
> +------------------------------------------------
Unneeded --- again, this is not a UNIX command guide --- and
wrong. You do not necessarily need to "launch with privileged
credentials" to do this anyway. As long as you can chmod the
directory, that is all that is needed.
> Next, give every team member read/write access to this repository. One
> easy way to do this is to give all the team members ssh access to the
> machine where the repository is hosted. If you don't want to give them a
> full shell on the machine, there is a restricted shell which only allows
> users to do git pushes and pulls; see gitlink:git-shell[1].
This part is a very good advice. It is git specific knowledge
new cvs migrants need to learn. Oops, the reason this part is
good is because it is from the original text --- no wonder ;-).
> -Put all the committers in the same group, and make the repository
> -writable by that group:
> +The following two commands will require admin privileges; first, enable
> +git-shell putting it on the trusted shells list of the system:
>
> ------------------------------------------------
> -$ chgrp -R $group /pub/my-repo.git
> +$ echo `which git-shell` >> /etc/shells
> +------------------------------------------------
Saying that /etc/shells may control what shells are allowed as
the login shell on many systems is probably a very good idea.
However, there is no need for an introductory UNIX guide that is
even WRONG. Why "echo `foo`" when just "foo" would work? Why
aren't you checking if /etc/shells already have git-shell
defined?
> +
> +Now, let's create the commit users:
> +
> +------------------------------------------------
> +$ useradd -g $group -s `which git-shell` $username
> ------------------------------------------------
>
> -Make sure committers have a umask of at most 027, so that the directories
> -they create are writable and searchable by other group members.
> +These users will be enabled to push on repositories owned by the group $group.
> +Later, you can give access to other projects simply by adding them to
> +other groups. Similarly, you can prevent access to repositories simply
> +removing those users from related groups.
The original text's point about umask does not apply to modern
git anymore, so the above lines can simply deleted. Almost
everything else you added to this hunk is unnecessary UNIX
guide.
> --- a/Documentation/git-init.txt
> +++ b/Documentation/git-init.txt
> @@ -101,6 +101,13 @@ $ git-add . <2>
> <2> add all existing file to the index
>
>
> +SHARED REPOSITORIES
> +-------------------
> +
> +Please refer to link:cvs-migration.html[git for CVS users], section "Setting Up
> +a Shared Repository", for details on how to set up shared repositories.
> +
> +
> Author
> ------
> Written by Linus Torvalds <torvalds@osdl.org>
This part is a good idea, but instead of putting it at the
bottom, it may make it more prominent to have this at the end of
option description for "--shared".
^ permalink raw reply
* Re: [PATCH 0/5] some shell portability fixes
From: Johannes Schindelin @ 2007-11-06 23:25 UTC (permalink / raw)
To: Mike Hommey; +Cc: Junio C Hamano, Ralf Wildenhues, git
In-Reply-To: <20071106210210.GA32159@glandium.org>
Hi,
On Tue, 6 Nov 2007, Mike Hommey wrote:
> On Tue, Nov 06, 2007 at 12:46:35PM -0800, Junio C Hamano wrote:
> > [5/5] Again, have you covered all of them? I am not opposed to
> > this one, although I am a bit curious who lacks -a/-o in
> > practice.
>
> Solaris's /bin/sh, but it already doesn't support $() and other stuff
> used all over the place in git, so it's not like it's changing anything.
>
> Maybe some other obscure old crappy shell ?
As Junio commented in the part you did not quote, there are better shells
in Solaris. Use those.
Ciao,
Dscho
^ permalink raw reply
* Re: git-rev-list diff options broken
From: Johannes Schindelin @ 2007-11-06 23:33 UTC (permalink / raw)
To: hanwen; +Cc: git
In-Reply-To: <f329bf540711061414k1627521bvaf4a7a06460cc3fd@mail.gmail.com>
Hi,
On Tue, 6 Nov 2007, Han-Wen Nienhuys wrote:
> the git-rev-list manpage says
>
>
> **
> Diff Formatting
> ~~~~~~~~~~~~~~~
>
> Below are listed options that control the formatting of diff output.
> Some of them are specific to gitlink:git-rev-list[1], however other diff
> options may be given. See gitlink:git-diff-files[1] for more options.
> **
>
> however, the source code says
>
>
> if ((!list &&
> (!(revs.tag_objects||revs.tree_objects||revs.blob_objects) &&
> !revs.pending.nr)) ||
> revs.diff)
> usage(rev_list_usage);
>
> so any attempt at showing diffs with git-rev-list will fail. What's
> the deal with this?
Probably you want to use "git log".
"git blame -L 585,589 builtin-rev-list.c" indicates that 8c1f0b44(Fix up
rev-list option parsing) was responsible, which in turn indicates that it
was intentional.
Hth,
Dscho
^ permalink raw reply
* Re: [PATCH 3/3] pretty=format: Avoid some expensive calculations when not needed
From: Johannes Schindelin @ 2007-11-06 23:36 UTC (permalink / raw)
To: René Scharfe; +Cc: Junio C Hamano, git
In-Reply-To: <4730EB4E.4080903@lsrfire.ath.cx>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 482 bytes --]
Hi,
On Tue, 6 Nov 2007, René Scharfe wrote:
> -char *interp_find_active(const char *orig,
> - const struct interp *interps, int ninterps)
> +void interp_find_active(const char *orig, struct interp *interps, int ninterps)
> {
> - char *result = xcalloc(1, ninterps);
> char c;
> int i;
>
> + for (i = 0; i < ninterps; i++)
> + interps[i].active = 0;
> +
> [...]
Funny.
I have the _exact_ same change in my repository. I only forgot to send
it, it seems.
Ciao,
Dscho
^ permalink raw reply
* Re: git-rev-list diff options broken
From: Han-Wen Nienhuys @ 2007-11-06 23:38 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0711062330220.4362@racer.site>
2007/11/6, Johannes Schindelin <Johannes.Schindelin@gmx.de>:
>
> Probably you want to use "git log".
>
> "git blame -L 585,589 builtin-rev-list.c" indicates that 8c1f0b44(Fix up
> rev-list option parsing) was responsible, which in turn indicates that it
> was intentional.
OK. So the man page needs fixing, right?
--
Han-Wen Nienhuys - hanwen@xs4all.nl - http://www.xs4all.nl/~hanwen
^ 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