* [StGIT PATCH 0/7] My patchqueue for 0.13.
From: Yann Dirson @ 2007-06-25 21:24 UTC (permalink / raw)
To: Catalin Marinas; +Cc: git
The following series contains the fixes that were scattered in the
middle of my refactoring series, but which I target at v0.13.
--
Yann Dirson <ydirson@altern.org> |
Debian-related: <dirson@debian.org> | Support Debian GNU/Linux:
| Freedom, Power, Stability, Gratis
http://ydirson.free.fr/ | Check <http://www.debian.org/>
^ permalink raw reply
* Re: [PATCH] git-rev-list: give better diagnostic for failed write
From: Linus Torvalds @ 2007-06-25 20:59 UTC (permalink / raw)
To: Jim Meyering; +Cc: git
In-Reply-To: <87r6nzu666.fsf@rho.meyering.net>
On Mon, 25 Jun 2007, Jim Meyering wrote:
>
> [this patch depends on the one I posted here:
> http://marc.info/?l=git&m=118280134031923&w=2 ]
>
> Without this patch, git-rev-list unnecessarily omits strerror(errno)
> from its diagnostic, upon write failure:
And this is a perfect example of what's wrong with the whole thing.
Dammit, how many times do I need to say this:
- If you want reliable errors, don't use stdio!
That fflush is there FOR A REASON. You removed it FOR A MUCH LESS
IMPORTANT REASON!
That fflush is there exactly because WE DO NOT WANT TO BUFFER the list of
commits, because that thing is meant very much to be used for pipelines,
and it's quite common that the receiving end is going to do something
asynchronous with the result, and can - and does - want the results as
soon as possible.
IOW, things like "gitk" use git-rev-list exactly to get the list of
commits, and they want that list *incrementally*. They don't want to wait
for git-rev-list to have filled up some 8kB buffer of commits. Especially
since generating those commits can be slow if we're talking about a big
tree and some path-limited stuff.
So for example, do something like
git rev-list HEAD -- drivers/char/drm/Makefile
and if you don't see the result scroll a line at a time on a slower
machine, there's something *wrong*.
Junio, I'm NAK'ing this very forcefully!
Jim: I don't know what I'm doing wrong, but I'm apparently not reaching
you. So let me try one more time:
- stdio really isn't very good with error handling
- if you use stdio, YOU HAD BETTER ACCEPT THAT
- don't screw up basic functionality in your *insane* quest to get stdio
to give you ENOSPC. It's not going to happen. Not that way. Just face
the fact that stdio *will* throw error numbers away.
The whole notion of "buffered IO" and "reliable errors" is simply not
something that goes well together.
Linus
^ permalink raw reply
* [PATCH] git-rev-list: give better diagnostic for failed write
From: Jim Meyering @ 2007-06-25 20:32 UTC (permalink / raw)
To: git
[this patch depends on the one I posted here:
http://marc.info/?l=git&m=118280134031923&w=2 ]
Without this patch, git-rev-list unnecessarily omits strerror(errno)
from its diagnostic, upon write failure:
$ ./git-rev-list --max-count=1 HEAD > /dev/full
fatal: write failure on standard output
With the patch, git reports the desired ENOSPC diagnostic:
fatal: write failure on standard output: No space left on device
* builtin-rev-list (show_commit): Don't fflush stdout here.
Instead, let the fclose in main do it, so there's a better
chance the underlying cause (errno) will be reported.
Signed-off-by: Jim Meyering <jim@meyering.net>
---
builtin-rev-list.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/builtin-rev-list.c b/builtin-rev-list.c
index 813aadf..62f0ba9 100644
--- a/builtin-rev-list.c
+++ b/builtin-rev-list.c
@@ -100,7 +100,6 @@ static void show_commit(struct commit *commit)
printf("%s%c", buf, hdr_termination);
free(buf);
}
- fflush(stdout);
if (commit->parents) {
free_commit_list(commit->parents);
commit->parents = NULL;
^ permalink raw reply related
* Re: [PATCH 2/2] Check for IO errors after running a command
From: Jim Meyering @ 2007-06-25 19:54 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.LFD.0.98.0706241010480.3593@woody.linux-foundation.org>
Linus Torvalds <torvalds@linux-foundation.org> wrote:
> This is trying to implement the strict IO error checks that Jim Meyering
> suggested, but explicitly limits it to just regular files. If a pipe gets
> closed on us, we shouldn't complain about it.
...
> Hmm? I'm not saying this is the only way to do this, but I think this is
> at least likely to be obviously just an improvement, and it leaves room
> for further tweaking of the logic if Jim or others find other cases that
> should be handled.
...
> + /* Check for ENOSPC and EIO errors.. */
> + if (!fstat(fileno(stdout), &st) && S_ISREG(st.st_mode)) {
> + if (ferror(stdout))
> + die("write failure on standard output");
> + if (fflush(stdout) || fclose(stdout))
> + die("write failure on standard output: %s", strerror(errno));
> + }
> + exit(0);
Here is a patch relative to "next", to restore some of the
functionality that was provided by my initially-proposed change.
>From 379aee16596d29b83c95068964c349399b9b9f47 Mon Sep 17 00:00:00 2001
From: Jim Meyering <jim@meyering.net>
Date: Mon, 25 Jun 2007 18:54:12 +0200
Subject: [PATCH] When detecting write failure, print strerror when possible.
Do not call "die" solely on the basis of ferror.
Instead, call both ferror and fclose, and *then* decide whether/how to die.
Without this patch, some commands unnecessarily omit strerror(errno)
when they fail:
$ ./git-ls-tree HEAD > /dev/full
fatal: write failure on standard output
With the patch, git reports the desired ENOSPC diagnostic:
fatal: write failure on standard output: No space left on device
FWIW, this version of close_stream is similar to the one
I included in another recent patch, but, is slightly cleaner.
Also, rather than returning zero or EOF, this one simply returns
zero or nonzero.
* git.c (close_stream): New function.
(run_command): Don't die solely because of ferror. Use close_stream.
Signed-off-by: Jim Meyering <jim@meyering.net>
---
git.c | 26 ++++++++++++++++++++++----
1 files changed, 22 insertions(+), 4 deletions(-)
diff --git a/git.c b/git.c
index b949cbb..b00bb1c 100644
--- a/git.c
+++ b/git.c
@@ -246,6 +246,21 @@ struct cmd_struct {
int option;
};
+static int
+close_stream(FILE *stream)
+{
+ int prev_fail = ferror(stream);
+ int fclose_fail = fclose(stream);
+
+ /* If there was a previous failure, but fclose succeeded,
+ clear errno, since ferror does not set it, and its value
+ may be unrelated to the ferror-reported failure. */
+ if (prev_fail && !fclose_fail)
+ errno = 0;
+
+ return prev_fail || fclose_fail;
+}
+
static int run_command(struct cmd_struct *p, int argc, const char **argv)
{
int status;
@@ -274,10 +289,13 @@ static int run_command(struct cmd_struct *p, int argc, const char **argv)
return 0;
/* Check for ENOSPC and EIO errors.. */
- if (ferror(stdout))
- die("write failure on standard output");
- if (fflush(stdout) || fclose(stdout))
- die("write failure on standard output: %s", strerror(errno));
+ if (close_stream(stdout)) {
+ if (errno == 0)
+ die("write failure on standard output");
+ else
+ die("write failure on standard output: %s",
+ strerror(errno));
+ }
return 0;
}
^ permalink raw reply related
* git for subversion users
From: Patrick Doyle @ 2007-06-25 19:48 UTC (permalink / raw)
To: git
Hello all,
I've read http://utsl.gen.nz/talks/git-svn/intro.html, "An
introduction to git-svn for Subversion/SVK users and deserters" and, I
guess I'm looking for a little more information.
It is possible that I am trying to use git and git-svn in a manner for
which they were not designed, so I appreciate any guidance that can be
provided. Also, since I'm running FC6, I have git version 1.5.0.6
installed, instead of the 1.5.2.2 that I see on the home page.
Perhaps that could be my problem.
Anyway, we have a subversion server set up to track our internal
software development. I would like to use git/git-svn so that I can
work offline, commit early and often, and occasionally synchronize to
our subversion baseline. Finally, at least one of our subversion
repositories (my own personal one), is not set up in the traditional
svn://host/repo/{trunk,tags,branches} format. It is organized as
svn://host/wpd/{project1,project2,project3}. Since it's my own
personal playground, I don't need branches, and tend to remember tags
just be commit number.
That's the long, boring setup. Now for the long boring question...
I started playing with a new project over the weekend, checkign in a
handful of commits in git, and now I want to import/export/push/pull
them to the subversion server.
Having read through the tutorial, I started with:
$ svn mkdir svn:///host/wpd/empty-project -m "Created empty project directory"
$ git-svn init svn:///host/wpd/empty-project
$ svn-git fetch
Now I have an empty directory into which I was hoping to "pull" my
changes from my weekend playground
$ git pull ~/playground/new-project
... (I get 7 new files, and, it looks like, their associated history)
Here's where I get stuck...
1) How can I remind myself of what I changed relative to what was in
the Subversion repository the last time I sync'd to it? Under my
current model of operation, I come in after a weekend/night away, do
"svn status" and "svn diff" to remind myself what's changed, and
commit those changes with appropriate log messages. I am hoping that
I can make the changes locally, commiting them with nice log messages
as I make the changes, and then "push" them to the subversion server
when convenient.
2) This is going to have some obvious problems when I work on other
projects shared with other developers. We know how to address this
with Subversion (good communication, updating the working copy prior
to a commit, resolving the minor conflicts, etc...) what can I expect
when my local repository is git?
3) If I try to commit my change with:
$ git-svn dcommit
I get an error
Commit 0e3e....
has no parent commit, and therefore nothing to diff against.
You should be working from a repository originally created by git-svn
and that's where I get confused. Is this a bug/feature of 1.5.0.2
that will disappear if I switched to 1.5.2.2?
Are there any other tips/resources for mixed mode operation
(centralized Subversion server, distributed git client(s))?
Thanks for any pointers.
--wpd
^ permalink raw reply
* [PATCH 2/2] Fix git-stripspace to process correctly long lines and spaces.
From: Carlos Rica @ 2007-06-25 19:28 UTC (permalink / raw)
To: git, Johannes.Schindelin, krh
Now the implementation gets more memory to store completely
each line before removing trailing spaces, and does it right
when the last line of the file ends with spaces and no newline
at the end.
Function stripspace needs again to be non-static in order to call
it from "builtin-tag.c" and the upcoming "builtin-commit.c".
A new parameter skip_comments was also added to the stripspace
function to optionally strips every shell #comment from the input,
needed for doing this task on those programs.
Signed-off-by: Carlos Rica <jasampler@gmail.com>
---
builtin-stripspace.c | 69 ++++++++++++++++++++++++++++++++-----------------
builtin.h | 1 +
2 files changed, 46 insertions(+), 24 deletions(-)
diff --git a/builtin-stripspace.c b/builtin-stripspace.c
index 62bd4b5..d8358e2 100644
--- a/builtin-stripspace.c
+++ b/builtin-stripspace.c
@@ -1,58 +1,79 @@
#include "builtin.h"
+#include "cache.h"
/*
- * Remove empty lines from the beginning and end.
+ * Remove trailing spaces from a line.
*
- * Turn multiple consecutive empty lines into just one
- * empty line. Return true if it is an incomplete line.
+ * If the line ends with newline, it will be removed too.
+ * Returns the new length of the string.
*/
-static int cleanup(char *line)
+static int cleanup(char *line, int len)
{
- int len = strlen(line);
+ if (len) {
+ if (line[len - 1] == '\n')
+ len--;
- if (len && line[len-1] == '\n') {
- if (len == 1)
- return 0;
- do {
- unsigned char c = line[len-2];
+ while (len) {
+ unsigned char c = line[len - 1];
if (!isspace(c))
break;
- line[len-2] = '\n';
len--;
- line[len] = 0;
- } while (len > 1);
- return 0;
+ }
+ line[len] = 0;
}
- return 1;
+ return len;
}
-static void stripspace(FILE *in, FILE *out)
+/*
+ * Remove empty lines from the beginning and end
+ * and also trailing spaces from every line.
+ *
+ * Turn multiple consecutive empty lines between paragraphs
+ * into just one empty line.
+ *
+ * If the input has only empty lines and spaces,
+ * no output will be produced.
+ *
+ * Enable skip_comments to skip every line starting with "#".
+ */
+void stripspace(FILE *in, FILE *out, int skip_comments)
{
int empties = -1;
- int incomplete = 0;
- char line[1024];
+ int alloc = 1024;
+ char *line = xmalloc(alloc);
+
+ while (fgets(line, alloc, in)) {
+ int len = strlen(line);
- while (fgets(line, sizeof(line), in)) {
- incomplete = cleanup(line);
+ while (len == alloc - 1 && line[len - 1] != '\n') {
+ alloc = alloc_nr(alloc);
+ line = xrealloc(line, alloc);
+ fgets(line + len, alloc - len, in);
+ len += strlen(line + len);
+ }
+
+ if (skip_comments && line[0] == '#')
+ continue;
+ len = cleanup(line, len);
/* Not just an empty line? */
- if (line[0] != '\n') {
+ if (len) {
if (empties > 0)
fputc('\n', out);
empties = 0;
fputs(line, out);
+ fputc('\n', out);
continue;
}
if (empties < 0)
continue;
empties++;
}
- if (incomplete)
- fputc('\n', out);
+ free(line);
}
int cmd_stripspace(int argc, const char **argv, const char *prefix)
{
- stripspace(stdin, stdout);
+ stripspace(stdin, stdout, 0);
return 0;
}
diff --git a/builtin.h b/builtin.h
index da4834c..661a92f 100644
--- a/builtin.h
+++ b/builtin.h
@@ -7,6 +7,7 @@ extern const char git_version_string[];
extern const char git_usage_string[];
extern void help_unknown_cmd(const char *cmd);
+extern void stripspace(FILE *in, FILE *out, int skip_comments);
extern int write_tree(unsigned char *sha1, int missing_ok, const char *prefix);
extern void prune_packed_objects(int);
--
1.5.0
^ permalink raw reply related
* Working on builtin-commit
From: Kristian Høgsberg @ 2007-06-25 19:27 UTC (permalink / raw)
To: git
Hi,
To avoid duplication of work, here's a short heads up. I've started
working on porting git-commit.sh (and status) to builtins. It's still
work in progress, and nowhere near a full-time effort, but to prevent
doing double work as with builtin-tag, I thought I'd let the list know.
I have a repo over here:
git://people.freedesktop.org/~krh/git.git
and the code is browsable here:
http://cgit.freedesktop.org/~krh/git.git
on the 'builtin-commit' branch.
It's just a snapshot of my current progress, so don't slam my lack of
error checking etc just yet. The work also needs to be split out in a
few patches (eg the read_pipe stuff). One thing that might be
interesting to discuss now, though, is my approach to option parsing. I
realize real men do this by hand using a lot of strcmp's, but I broke
down and wrote a little option parser. You give it a pointer to argv
and an array of these:
struct option {
enum option_type type;
const char *long_name;
char short_name;
void *value;
};
with
enum option_type {
OPTION_NONE,
OPTION_STRING,
OPTION_INTEGER,
OPTION_LAST
};
and it goes and parses the command line. As the shell script, it
accepts all abreviations of long options, eg all of --messa doit,
--message=doit, -mdoit, -m doit, for the long_name="message";
short_name='m' case. Ambiguous abbreviations are resolved according to
the order the options appears in the array. I've kept it as simple as
possible and it works well for builtin-commit.c. It's obviously useful
for other commands, but my goal here is to get git-commit.sh ported to
C, not to host a flamewar over the option parser API and whether we
should port other builtins.
cheers,
Kristian
^ permalink raw reply
* [PATCH 1/2] Add test script for git-stripspace.
From: Carlos Rica @ 2007-06-25 19:24 UTC (permalink / raw)
To: git, Johannes.Schindelin, krh
These tests check some features that git-stripspace already has
and those that it should manage well: Removing trailing spaces
from lines, removing blank lines at the beginning and end,
unifying multiple lines between paragraphs, doing the correct
when there is no newline at the last line, etc.
It seems that the implementation needs to save the whole line
in memory to be able to manage correctly long lines with
text and spaces conveniently distribuited on them.
Signed-off-by: Carlos Rica <jasampler@gmail.com>
---
t/t0030-stripspace.sh | 355
+++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 355 insertions(+), 0 deletions(-)
create mode 100644 t/t0030-stripspace.sh
diff --git a/t/t0030-stripspace.sh b/t/t0030-stripspace.sh
new file mode 100644
index 0000000..abd82d7
--- /dev/null
+++ b/t/t0030-stripspace.sh
@@ -0,0 +1,355 @@
+#!/bin/sh
+#
+# Copyright (c) 2007 Carlos Rica
+#
+
+test_description='git-stripspace'
+
+. ./test-lib.sh
+
+t40='A quick brown fox jumps over the lazy do'
+s40=' '
+sss="$s40$s40$s40$s40$s40$s40$s40$s40$s40$s40" # 400
+ttt="$t40$t40$t40$t40$t40$t40$t40$t40$t40$t40" # 400
+
+test_expect_success \
+ 'long lines without spaces should be unchanged' '
+ echo "$ttt" >expect &&
+ cat expect | git-stripspace >actual &&
+ git diff expect actual &&
+
+ echo "$ttt$ttt" >expect &&
+ cat expect | git-stripspace >actual &&
+ git diff expect actual &&
+
+ echo "$ttt$ttt$ttt" >expect &&
+ cat expect | git-stripspace >actual &&
+ git diff expect actual &&
+
+ echo "$ttt$ttt$ttt$ttt" >expect &&
+ cat expect | git-stripspace >actual &&
+ git diff expect actual
+'
+
+test_expect_success \
+ 'lines with spaces at the beginning should be unchanged' '
+ echo "$sss$ttt" >expect &&
+ cat expect | git-stripspace >actual &&
+ git diff expect actual &&
+
+ echo "$sss$sss$ttt" >expect &&
+ cat expect | git-stripspace >actual &&
+ git diff expect actual &&
+
+ echo "$sss$sss$sss$ttt" >expect &&
+ cat expect | git-stripspace >actual &&
+ git diff expect actual
+'
+
+test_expect_success \
+ 'lines with intermediate spaces should be unchanged' '
+ echo "$ttt$sss$ttt" >expect &&
+ cat expect | git-stripspace >actual &&
+ git diff expect actual &&
+
+ echo "$ttt$sss$sss$ttt" >expect &&
+ cat expect | git-stripspace >actual &&
+ git diff expect actual
+'
+
+test_expect_success \
+ 'consecutive blank lines should be unified' '
+ printf "$ttt\n\n$ttt\n" > expect &&
+ printf "$ttt\n\n\n\n\n$ttt\n" | git-stripspace >actual &&
+ git diff expect actual &&
+
+ printf "$ttt$ttt\n\n$ttt\n" > expect &&
+ printf "$ttt$ttt\n\n\n\n\n$ttt\n" | git-stripspace >actual &&
+ git diff expect actual &&
+
+ printf "$ttt$ttt$ttt\n\n$ttt\n" > expect &&
+ printf "$ttt$ttt$ttt\n\n\n\n\n$ttt\n" | git-stripspace >actual &&
+ git diff expect actual &&
+
+ printf "$ttt\n\n$ttt\n" > expect &&
+ printf "$ttt\n\n\n\n\n$ttt\n" | git-stripspace >actual &&
+ git diff expect actual &&
+
+ printf "$ttt\n\n$ttt$ttt\n" > expect &&
+ printf "$ttt\n\n\n\n\n$ttt$ttt\n" | git-stripspace >actual &&
+ git diff expect actual &&
+
+ printf "$ttt\n\n$ttt$ttt$ttt\n" > expect &&
+ printf "$ttt\n\n\n\n\n$ttt$ttt$ttt\n" | git-stripspace >actual &&
+ git diff expect actual
+'
+
+test_expect_success \
+ 'consecutive blank lines at the beginning should be removed' '
+ printf "" > expect &&
+ printf "\n" | git-stripspace >actual &&
+ git diff expect actual &&
+
+ printf "" > expect &&
+ printf "\n\n\n" | git-stripspace >actual &&
+ git diff expect actual &&
+
+ printf "" > expect &&
+ printf "$sss\n$sss\n$sss\n" | git-stripspace >actual &&
+ git diff expect actual &&
+
+ printf "" > expect &&
+ printf "$sss$sss\n$sss\n\n" | git-stripspace >actual &&
+ git diff expect actual &&
+
+ printf "" > expect &&
+ printf "\n$sss\n$sss$sss\n" | git-stripspace >actual &&
+ git diff expect actual &&
+
+ printf "" > expect &&
+ printf "$sss$sss$sss$sss\n\n\n" | git-stripspace >actual &&
+ git diff expect actual &&
+
+ printf "" > expect &&
+ printf "\n$sss$sss$sss$sss\n\n" | git-stripspace >actual &&
+ git diff expect actual &&
+
+ printf "" > expect &&
+ printf "\n\n$sss$sss$sss$sss\n" | git-stripspace >actual &&
+ git diff expect actual &&
+
+ printf "$ttt\n" > expect &&
+ printf "\n$ttt\n" | git-stripspace >actual &&
+ git diff expect actual &&
+
+ printf "$ttt\n" > expect &&
+ printf "\n\n\n$ttt\n" | git-stripspace >actual &&
+ git diff expect actual &&
+
+ printf "$ttt$ttt\n" > expect &&
+ printf "\n\n\n$ttt$ttt\n" | git-stripspace >actual &&
+ git diff expect actual &&
+
+ printf "$ttt$ttt$ttt\n" > expect &&
+ printf "\n\n\n$ttt$ttt$ttt\n" | git-stripspace >actual &&
+ git diff expect actual &&
+
+ printf "$ttt$ttt$ttt$ttt\n" > expect &&
+ printf "\n\n\n$ttt$ttt$ttt$ttt\n" | git-stripspace >actual &&
+ git diff expect actual &&
+
+ printf "$ttt\n" > expect &&
+ printf "$sss\n$sss\n$sss\n$ttt\n" | git-stripspace >actual &&
+ git diff expect actual &&
+
+ printf "$ttt\n" > expect &&
+ printf "\n$sss\n$sss$sss\n$ttt\n" | git-stripspace >actual &&
+ git diff expect actual &&
+
+ printf "$ttt\n" > expect &&
+ printf "$sss$sss\n$sss\n\n$ttt\n" | git-stripspace >actual &&
+ git diff expect actual &&
+
+ printf "$ttt\n" > expect &&
+ printf "$sss$sss$sss\n\n\n$ttt\n" | git-stripspace >actual &&
+ git diff expect actual &&
+
+ printf "$ttt\n" > expect &&
+ printf "\n$sss$sss$sss\n\n$ttt\n" | git-stripspace >actual &&
+ git diff expect actual &&
+
+ printf "$ttt\n" > expect &&
+ printf "\n\n$sss$sss$sss\n$ttt\n" | git-stripspace >actual &&
+ git diff expect actual
+'
+
+test_expect_success \
+ 'consecutive blank lines at the end should be removed' '
+ printf "$ttt\n" > expect &&
+ printf "$ttt\n\n" | git-stripspace >actual &&
+ git diff expect actual &&
+
+ printf "$ttt\n" > expect &&
+ printf "$ttt\n\n\n\n" | git-stripspace >actual &&
+ git diff expect actual &&
+
+ printf "$ttt$ttt\n" > expect &&
+ printf "$ttt$ttt\n\n\n\n" | git-stripspace >actual &&
+ git diff expect actual &&
+
+ printf "$ttt$ttt$ttt\n" > expect &&
+ printf "$ttt$ttt$ttt\n\n\n\n" | git-stripspace >actual &&
+ git diff expect actual &&
+
+ printf "$ttt$ttt$ttt$ttt\n" > expect &&
+ printf "$ttt$ttt$ttt$ttt\n\n\n\n" | git-stripspace >actual &&
+ git diff expect actual &&
+
+ printf "$ttt\n" > expect &&
+ printf "$ttt\n$sss\n$sss\n$sss\n" | git-stripspace >actual &&
+ git diff expect actual &&
+
+ printf "$ttt\n" > expect &&
+ printf "$ttt\n\n$sss\n$sss$sss\n" | git-stripspace >actual &&
+ git diff expect actual &&
+
+ printf "$ttt\n" > expect &&
+ printf "$ttt\n$sss$sss\n$sss\n\n" | git-stripspace >actual &&
+ git diff expect actual &&
+
+ printf "$ttt\n" > expect &&
+ printf "$ttt\n$sss$sss$sss\n\n\n" | git-stripspace >actual &&
+ git diff expect actual &&
+
+ printf "$ttt\n" > expect &&
+ printf "$ttt\n\n$sss$sss$sss\n\n" | git-stripspace >actual &&
+ git diff expect actual &&
+
+ printf "$ttt\n" > expect &&
+ printf "$ttt\n\n\n$sss$sss$sss\n" | git-stripspace >actual &&
+ git diff expect actual
+'
+
+test_expect_success \
+ 'text without newline at end should end with newline' '
+ test `printf "$ttt" | git-stripspace | wc -l` -gt 0 &&
+ test `printf "$ttt$ttt" | git-stripspace | wc -l` -gt 0 &&
+ test `printf "$ttt$ttt$ttt" | git-stripspace | wc -l` -gt 0 &&
+ test `printf "$ttt$ttt$ttt$ttt" | git-stripspace | wc -l` -gt 0
+'
+
+# text plus spaces at the end:
+
+test_expect_success \
+ 'text plus spaces without newline at end should end with newline' '
+ test `printf "$ttt$sss" | git-stripspace | wc -l` -gt 0 &&
+ test `printf "$ttt$ttt$sss" | git-stripspace | wc -l` -gt 0 &&
+ test `printf "$ttt$ttt$ttt$sss" | git-stripspace | wc -l` -gt 0
+ test `printf "$ttt$sss$sss" | git-stripspace | wc -l` -gt 0 &&
+ test `printf "$ttt$ttt$sss$sss" | git-stripspace | wc -l` -gt 0 &&
+ test `printf "$ttt$sss$sss$sss" | git-stripspace | wc -l` -gt 0
+'
+
+test_expect_failure \
+ 'text plus spaces without newline at end should not show spaces' '
+ printf "$ttt$sss" | git-stripspace | grep -q " " ||
+ printf "$ttt$ttt$sss" | git-stripspace | grep -q " " ||
+ printf "$ttt$ttt$ttt$sss" | git-stripspace | grep -q " " ||
+ printf "$ttt$sss$sss" | git-stripspace | grep -q " " ||
+ printf "$ttt$ttt$sss$sss" | git-stripspace | grep -q " " ||
+ printf "$ttt$sss$sss$sss" | git-stripspace | grep -q " "
+'
+
+test_expect_success \
+ 'text plus spaces without newline should show the correct lines' '
+ printf "$ttt\n" >expect &&
+ printf "$ttt$sss" | git-stripspace >actual &&
+ git diff expect actual &&
+
+ printf "$ttt\n" >expect &&
+ printf "$ttt$sss$sss" | git-stripspace >actual &&
+ git diff expect actual &&
+
+ printf "$ttt\n" >expect &&
+ printf "$ttt$sss$sss$sss" | git-stripspace >actual &&
+ git diff expect actual &&
+
+ printf "$ttt$ttt\n" >expect &&
+ printf "$ttt$ttt$sss" | git-stripspace >actual &&
+ git diff expect actual &&
+
+ printf "$ttt$ttt\n" >expect &&
+ printf "$ttt$ttt$sss$sss" | git-stripspace >actual &&
+ git diff expect actual &&
+
+ printf "$ttt$ttt$ttt\n" >expect &&
+ printf "$ttt$ttt$ttt$sss" | git-stripspace >actual &&
+ git diff expect actual
+'
+
+test_expect_failure \
+ 'text plus spaces at end should not show spaces' '
+ echo "$ttt$sss" | git-stripspace | grep -q " " ||
+ echo "$ttt$ttt$sss" | git-stripspace | grep -q " " ||
+ echo "$ttt$ttt$ttt$sss" | git-stripspace | grep -q " " ||
+ echo "$ttt$sss$sss" | git-stripspace | grep -q " " ||
+ echo "$ttt$ttt$sss$sss" | git-stripspace | grep -q " " ||
+ echo "$ttt$sss$sss$sss" | git-stripspace | grep -q " "
+'
+
+test_expect_success \
+ 'text plus spaces at end should be cleaned and newline must remain' '
+ echo "$ttt" >expect &&
+ echo "$ttt$sss" | git-stripspace >actual &&
+ git diff expect actual &&
+
+ echo "$ttt" >expect &&
+ echo "$ttt$sss$sss" | git-stripspace >actual &&
+ git diff expect actual &&
+
+ echo "$ttt" >expect &&
+ echo "$ttt$sss$sss$sss" | git-stripspace >actual &&
+ git diff expect actual &&
+
+ echo "$ttt$ttt" >expect &&
+ echo "$ttt$ttt$sss" | git-stripspace >actual &&
+ git diff expect actual &&
+
+ echo "$ttt$ttt" >expect &&
+ echo "$ttt$ttt$sss$sss" | git-stripspace >actual &&
+ git diff expect actual &&
+
+ echo "$ttt$ttt$ttt" >expect &&
+ echo "$ttt$ttt$ttt$sss" | git-stripspace >actual &&
+ git diff expect actual
+'
+
+# spaces only:
+
+test_expect_success \
+ 'spaces with newline at end should be replaced with empty string' '
+ printf "" >expect &&
+
+ echo | git-stripspace >actual &&
+ git diff expect actual &&
+
+ echo "$sss" | git-stripspace >actual &&
+ git diff expect actual &&
+
+ echo "$sss$sss" | git-stripspace >actual &&
+ git diff expect actual &&
+
+ echo "$sss$sss$sss" | git-stripspace >actual &&
+ git diff expect actual &&
+
+ echo "$sss$sss$sss$sss" | git-stripspace >actual &&
+ git diff expect actual
+'
+
+test_expect_failure \
+ 'spaces without newline at end should not show spaces' '
+ printf "" | git-stripspace | grep -q " " ||
+ printf "$sss" | git-stripspace | grep -q " " ||
+ printf "$sss$sss" | git-stripspace | grep -q " " ||
+ printf "$sss$sss$sss" | git-stripspace | grep -q " " ||
+ printf "$sss$sss$sss$sss" | git-stripspace | grep -q " "
+'
+
+test_expect_success \
+ 'spaces without newline at end should be replaced with empty string' '
+ printf "" >expect &&
+
+ printf "" | git-stripspace >actual &&
+ git diff expect actual
+
+ printf "$sss$sss" | git-stripspace >actual &&
+ git diff expect actual
+
+ printf "$sss$sss$sss" | git-stripspace >actual &&
+ git diff expect actual
+
+ printf "$sss$sss$sss$sss" | git-stripspace >actual &&
+ git diff expect actual
+'
+
+test_done
--
1.5.0
^ permalink raw reply related
* Re: DWIM ref names for push/fetch
From: Julian Phillips @ 2007-06-25 18:45 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0706251216210.4740@iabervon.org>
On Mon, 25 Jun 2007, Daniel Barkalow wrote:
> On Sun, 24 Jun 2007, Junio C Hamano wrote:
>
>> Daniel Barkalow <barkalow@iabervon.org> writes:
>>
>>> I was actually thinking exclusively of the matching of strings like "HEAD"
>>> or "origin/next" or "master" to refs from the list of available refs. It
>>> seems to me like the push code does a better job of handling the same
>>> sorts of things that get_sha1() handles.
>>>
>>> In particular, the handling of "refs/my/funny/thing" is really wrong: it
>>> gets treated as refs/heads/refs/my/funny/thing.
>>
>> git-parse-remote.sh::canon_refs_list_for_fetch() seems to say
>> otherwise, though.
>>
>> - When unspecified, or explicitly spelled HEAD, take HEAD;
>> - Anything that begins with refs/, use it as is;
>> - Anything that begins with heads/, tags/, remotes/, assume
>> it is a branch, a tag, or a tracking branch;
>> - Otherwise assume a branch;
>>
>> So I suspect refs/my/funny/thing is covered by the second rule.
>
> Ah, okay. I think a few bits got lost somewhat in Julian's translation to
> C. I agree with the first three rules there, and with the last rule being
> the last rule, and sticking more things in between those sets is easy
> enough.
Just for the record as it were, the difference between my C code and
git-parse-remote.sh is simply that commit 96f12b5 changed the shell script
behaviour after I had already started, and translating the code to C was
hard enough without also trying to track a moving target.
Effectively Daniel is working slightly in the past, and has spotted the
same issue that Alex has already fixed.
Mea Culpa. Sorry.
>
>> But I do agree "otherwise assume a branch" part has huge room
>> for improvement. Especially...
>>
>>> I think that "origin/next"
>>> should also be assumed to be refs/remotes/origin/next instead of
>>> refs/heads/origin/next, at least if you have refs/remotes/origin/ and not
>>> refs/heads/origin/.
>>
>> ... I think that makes perfect sense -- the code should
>> interpret your example as a request to start using a new
>> tracking branch refs/remotes/origin/next.
>
> Currently, it doesn't even notice if you've got the tracking branch
> already. Should it have some rule to prefer things that exist over things
> that don't?
>
> When refs/remotes/origin/next doesn't exist, should it require that
> refs/remotes/origin/ already exist?
It should at least require that a remote called origin exists perhaps?
--
Julian
---
<rcw> those apparently-bacteria-like multicolor worms coming out of
microsoft's backorifice
<rcw> that's the backoffice logo
^ permalink raw reply
* [PATCH 3/3] Teach rebase -i about --preserve-merges
From: Johannes Schindelin @ 2007-06-25 17:59 UTC (permalink / raw)
To: git, gitster
The option "-p" (or long "--preserve-merges") makes it possible to
rebase side branches including merges, without straightening the
history.
Example:
X
\
A---M---B
/
---o---O---P---Q
When the current HEAD is "B", "git rebase -i -p --onto Q O" will yield
X
\
---o---O---P---Q---A'---M'---B'
Note that this will
- _not_ touch X [*1*], it does
- _not_ work without the --interactive flag [*2*], it does
- _not_ guess the type of the merge, but blindly uses recursive or
whatever strategy you provided with "-s <strategy>" for all merges it
has to redo, and it does
- _not_ make use of the original merge commit via git-rerere.
*1*: only commits which reach a merge base between <upstream> and HEAD
are reapplied. The others are kept as-are.
*2*: git-rebase without --interactive is inherently patch based (at
least at the moment), and therefore merges cannot be preserved.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
This (especially the part about passing the message to git merge,
you know, correctly quoted and stuff) could use a couple of
eyeball pairs.
Documentation/git-rebase.txt | 23 ++++++++-
git-rebase--interactive.sh | 110 +++++++++++++++++++++++++++++++++++++++-
t/t3404-rebase-interactive.sh | 22 ++++++++
3 files changed, 151 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 2e3363a..96907d4 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -9,7 +9,7 @@ SYNOPSIS
--------
[verse]
'git-rebase' [-i | --interactive] [-v | --verbose] [--merge] [-C<n>]
- [--onto <newbase>] <upstream> [<branch>]
+ [-p | --preserve-merges] [--onto <newbase>] <upstream> [<branch>]
'git-rebase' --continue | --skip | --abort
DESCRIPTION
@@ -213,6 +213,10 @@ OPTIONS
Make a list of the commits which are about to be rebased. Let the
user edit that list before rebasing.
+-p, \--preserve-merges::
+ Instead of ignoring merges, try to recreate them. This option
+ only works in interactive mode.
+
include::merge-strategies.txt[]
NOTES
@@ -304,6 +308,23 @@ $ git rebase -i HEAD~5
And move the first patch to the end of the list.
+You might want to preserve merges, if you have a history like this:
+
+------------------
+ X
+ \
+ A---M---B
+ /
+---o---O---P---Q
+------------------
+
+Suppose you want to rebase the side branch starting at "A" to "Q". Make
+sure that the current HEAD is "B", and call
+
+-----------------------------
+$ git rebase -i -p --onto Q O
+-----------------------------
+
Authors
------
Written by Junio C Hamano <junkio@cox.net> and
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index fb93e13..73cbcde 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -10,7 +10,8 @@
# The original idea comes from Eric W. Biederman, in
# http://article.gmane.org/gmane.comp.version-control.git/22407
-USAGE='(--continue | --abort | --skip | [--onto <branch>] <upstream> [<branch>])'
+USAGE='(--continue | --abort | --skip | [--preserve-merges] [--verbose]
+ [--onto <branch>] <upstream> [<branch>])'
. git-sh-setup
require_work_tree
@@ -18,6 +19,8 @@ require_work_tree
DOTEST="$GIT_DIR/.dotest-merge"
TODO="$DOTEST"/todo
DONE="$DOTEST"/done
+REWRITTEN="$DOTEST"/rewritten
+PRESERVE_MERGES=
STRATEGY=
VERBOSE=
@@ -68,6 +71,8 @@ die_abort () {
pick_one () {
case "$1" in -n) sha1=$2 ;; *) sha1=$1 ;; esac
git rev-parse --verify $sha1 || die "Invalid commit name: $sha1"
+ test -d "$REWRITTEN" &&
+ pick_one_preserving_merges "$@" && return
parent_sha1=$(git rev-parse --verify $sha1^ 2>/dev/null)
current_sha1=$(git rev-parse --verify HEAD)
if [ $current_sha1 = $parent_sha1 ]; then
@@ -79,6 +84,75 @@ pick_one () {
fi
}
+pick_one_preserving_merges () {
+ case "$1" in -n) sha1=$2 ;; *) sha1=$1 ;; esac
+ sha1=$(git rev-parse $sha1)
+
+ if [ -f "$DOTEST"/current-commit ]
+ then
+ current_commit=$(cat "$DOTEST"/current-commit) &&
+ git rev-parse HEAD > "$REWRITTEN"/$current_commit &&
+ rm "$DOTEST"/current-commit ||
+ die "Cannot write current commit's replacement sha1"
+ fi
+
+ # rewrite parents; if none were rewritten, we can fast-forward.
+ fast_forward=t
+ preserve=t
+ new_parents=
+ for p in $(git rev-list --parents -1 $sha1 | cut -d\ -f2-)
+ do
+ if [ -f "$REWRITTEN"/$p ]
+ then
+ preserve=f
+ new_p=$(cat "$REWRITTEN"/$p)
+ test $p != $new_p && fast_forward=f
+ case "$new_parents" in
+ *$new_p*)
+ ;; # do nothing; that parent is already there
+ *)
+ new_parents="$new_parents $new_p"
+ esac
+ fi
+ done
+ case $fast_forward in
+ t)
+ echo "Fast forward to $sha1"
+ test $preserve=f && echo $sha1 > "$REWRITTEN"/$sha1
+ ;;
+ f)
+ test "a$1" = a-n && die "Refusing to squash a merge: $sha1"
+
+ first_parent=$(expr "$new_parents" : " \([^ ]*\)")
+ # detach HEAD to current parent
+ git checkout $first_parent 2> /dev/null ||
+ die "Cannot move HEAD to $first_parent"
+
+ echo $sha1 > "$DOTEST"/current-commit
+ case "$new_parents" in
+ \ *\ *)
+ # redo merge
+ author_script=$(get_author_ident_from_commit $sha1)
+ eval "$author_script"
+ msg="$(git cat-file commit $sha1 | \
+ sed -e '1,/^$/d' -e "s/[\"\\]/\\\\&/g")"
+ # NEEDSWORK: give rerere a chance
+ if ! git merge $STRATEGY -m "$msg" $new_parents
+ then
+ echo "$msg" > "$GIT_DIR"/MERGE_MSG
+ warn Error redoing merge $sha1
+ warn
+ warn After fixup, please use
+ die "$author_script git commit"
+ fi
+ ;;
+ *)
+ git cherry-pick $STRATEGY "$@" ||
+ die_with_patch $sha1 "Could not pick $sha1"
+ esac
+ esac
+}
+
do_next () {
read command sha1 rest < "$TODO"
case "$command" in
@@ -155,7 +229,15 @@ do_next () {
HEADNAME=$(cat "$DOTEST"/head-name) &&
OLDHEAD=$(cat "$DOTEST"/head) &&
SHORTONTO=$(git rev-parse --short $(cat "$DOTEST"/onto)) &&
- NEWHEAD=$(git rev-parse HEAD) &&
+ if [ -d "$REWRITTEN" ]
+ then
+ test -f "$DOTEST"/current-commit &&
+ current_commit=$(cat "$DOTEST"/current-commit) &&
+ git rev-parse HEAD > "$REWRITTEN"/$current_commit
+ NEWHEAD=$(cat "$REWRITTEN"/$OLDHEAD)
+ else
+ NEWHEAD=$(git rev-parse HEAD)
+ fi &&
message="$GIT_REFLOG_ACTION: $HEADNAME onto $SHORTONTO)" &&
git update-ref -m "$message" $HEADNAME $NEWHEAD $OLDHEAD &&
git symbolic-ref HEAD $HEADNAME &&
@@ -226,6 +308,9 @@ do
-v|--verbose)
VERBOSE=t
;;
+ -p|--preserve-merges)
+ PRESERVE_MERGES=t
+ ;;
-i|--interactive)
# yeah, we know
;;
@@ -274,6 +359,25 @@ do
echo $UPSTREAM > "$DOTEST"/upstream
echo $ONTO > "$DOTEST"/onto
test t = "$VERBOSE" && : > "$DOTEST"/verbose
+ if [ t = "$PRESERVE_MERGES" ]
+ then
+ # $REWRITTEN contains files for each commit that is
+ # reachable by at least one merge base of $HEAD and
+ # $UPSTREAM. They are not necessarily rewritten, but
+ # their children might be.
+ # This ensures that commits on merged, but otherwise
+ # unrelated side branches are left alone. (Think "X"
+ # in the man page's example.)
+ mkdir "$REWRITTEN" &&
+ for c in $(git merge-base --all $HEAD $UPSTREAM)
+ do
+ echo $ONTO > "$REWRITTEN"/$c ||
+ die "Could not init rewritten commits"
+ done
+ MERGES_OPTION=
+ else
+ MERGES_OPTION=--no-merges
+ fi
SHORTUPSTREAM=$(git rev-parse --short $UPSTREAM)
SHORTHEAD=$(git rev-parse --short $HEAD)
@@ -286,7 +390,7 @@ do
# edit = use commit, but stop for amending
# squash = use commit, but meld into previous commit
EOF
- git rev-list --no-merges --pretty=oneline --abbrev-commit \
+ git rev-list $MERGES_OPTION --pretty=oneline --abbrev-commit \
--abbrev=7 --reverse $UPSTREAM..$HEAD | \
sed "s/^/pick /" >> "$TODO"
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 9f12bb9..883cf29 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -166,4 +166,26 @@ test_expect_success 'retain authorship when squashing' '
git show HEAD | grep "^Author: Nitfol"
'
+test_expect_success 'preserve merges with -p' '
+ git checkout -b to-be-preserved master^ &&
+ : > unrelated-file &&
+ git add unrelated-file &&
+ test_tick &&
+ git commit -m "unrelated" &&
+ git checkout -b to-be-rebased master &&
+ echo B > file1 &&
+ test_tick &&
+ git commit -m J file1 &&
+ test_tick &&
+ git merge to-be-preserved &&
+ echo C > file1 &&
+ test_tick &&
+ git commit -m K file1 &&
+ git rebase -i -p --onto branch1 master &&
+ test $(git rev-parse HEAD^^2) = $(git rev-parse to-be-preserved) &&
+ test $(git rev-parse HEAD~3) = $(git rev-parse branch1) &&
+ test $(git show HEAD:file1) = C &&
+ test $(git show HEAD~2:file1) = B
+'
+
test_done
--
1.5.2.2.3172.ge55a1-dirty
^ permalink raw reply related
* [PATCH 2/3] rebase -i: provide reasonable reflog for the rebased branch
From: Johannes Schindelin @ 2007-06-25 17:58 UTC (permalink / raw)
To: git, gitster, j.sixt
If your rebase succeeded, the HEAD's reflog will still show the whole
mess, but "<branchname>@{1}" now shows the state _before_ the rebase,
so that you can reset (or compare) the original and the rebased
revisions more easily.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
Johannes, how about this? ;-)
git-rebase--interactive.sh | 10 ++++++++--
t/t3404-rebase-interactive.sh | 4 ++++
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index b95fe86..fb93e13 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -151,8 +151,14 @@ do_next () {
esac
test -s "$TODO" && return
- HEAD=$(git rev-parse HEAD)
- HEADNAME=$(cat "$DOTEST"/head-name)
+ comment_for_reflog finish &&
+ HEADNAME=$(cat "$DOTEST"/head-name) &&
+ OLDHEAD=$(cat "$DOTEST"/head) &&
+ SHORTONTO=$(git rev-parse --short $(cat "$DOTEST"/onto)) &&
+ NEWHEAD=$(git rev-parse HEAD) &&
+ message="$GIT_REFLOG_ACTION: $HEADNAME onto $SHORTONTO)" &&
+ git update-ref -m "$message" $HEADNAME $NEWHEAD $OLDHEAD &&
+ git symbolic-ref HEAD $HEADNAME &&
rm -rf "$DOTEST" &&
warn "Successfully rebased and updated $HEADNAME."
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 19a3a8e..9f12bb9 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -99,6 +99,10 @@ test_expect_success 'rebase on top of a non-conflicting commit' '
test $(git rev-parse I) = $(git rev-parse HEAD~2)
'
+test_expect_success 'reflog for the branch shows state before rebase' '
+ test $(git rev-parse branch1@{1}) = $(git rev-parse original-branch1)
+'
+
test_expect_success 'exchange two commits' '
FAKE_LINES="2 1" git rebase -i HEAD~2 &&
test H = $(git cat-file commit HEAD^ | tail -n 1) &&
--
1.5.2.2.3172.ge55a1-dirty
^ permalink raw reply related
* [PATCH 1/3] rebase -i: several cleanups
From: Johannes Schindelin @ 2007-06-25 17:56 UTC (permalink / raw)
To: git, gitster
Support "--verbose" in addition to "-v", show short names in the list
comment, clean up if there is nothing to do, and add several "test_ticks"
in the test script.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
git-rebase--interactive.sh | 19 +++++++++++++++----
t/t3404-rebase-interactive.sh | 2 ++
2 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index ab36572..b95fe86 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -60,6 +60,11 @@ die_with_patch () {
die "$2"
}
+die_abort () {
+ rm -rf "$DOTEST" 2> /dev/null
+ die "$1"
+}
+
pick_one () {
case "$1" in -n) sha1=$2 ;; *) sha1=$1 ;; esac
git rev-parse --verify $sha1 || die "Invalid commit name: $sha1"
@@ -212,7 +217,7 @@ do
-C*)
die "Interactive rebase uses merge, so $1 does not make sense"
;;
- -v)
+ -v|--verbose)
VERBOSE=t
;;
-i|--interactive)
@@ -264,8 +269,11 @@ do
echo $ONTO > "$DOTEST"/onto
test t = "$VERBOSE" && : > "$DOTEST"/verbose
+ SHORTUPSTREAM=$(git rev-parse --short $UPSTREAM)
+ SHORTHEAD=$(git rev-parse --short $HEAD)
+ SHORTONTO=$(git rev-parse --short $ONTO)
cat > "$TODO" << EOF
-# Rebasing $UPSTREAM..$HEAD onto $ONTO
+# Rebasing $SHORTUPSTREAM)..$SHORTHEAD onto $SHORTONTO
#
# Commands:
# pick = use commit
@@ -277,13 +285,16 @@ EOF
sed "s/^/pick /" >> "$TODO"
test -z "$(grep -ve '^$' -e '^#' < $TODO)" &&
- die "Nothing to do"
+ die_abort "Nothing to do"
cp "$TODO" "$TODO".backup
${VISUAL:-${EDITOR:-vi}} "$TODO" ||
die "Could not execute editor"
- git reset --hard $ONTO && do_rest
+ test -z "$(grep -ve '^$' -e '^#' < $TODO)" &&
+ die_abort "Nothing to do"
+
+ git checkout $ONTO && do_rest
esac
shift
done
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 48aa8ea..19a3a8e 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -140,6 +140,7 @@ test_expect_success 'abort' '
test_expect_success 'retain authorship' '
echo A > file7 &&
git add file7 &&
+ test_tick &&
GIT_AUTHOR_NAME="Twerp Snog" git commit -m "different author" &&
git tag twerp &&
git rebase -i --onto master HEAD^ &&
@@ -149,6 +150,7 @@ test_expect_success 'retain authorship' '
test_expect_success 'squash' '
git reset --hard twerp &&
echo B > file7 &&
+ test_tick &&
GIT_AUTHOR_NAME="Nitfol" git commit -m "nitfol" file7 &&
echo "******************************" &&
FAKE_LINES="1 squash 2" git rebase -i --onto master HEAD~2 &&
--
1.5.2.2.3172.ge55a1-dirty
^ permalink raw reply related
* Re: Darcs
From: Bu Bacoo @ 2007-06-25 16:54 UTC (permalink / raw)
To: Florian Weimer; +Cc: git
In-Reply-To: <87vedcqna7.fsf@mid.deneb.enyo.de>
Florian, what are you moving to? To GIT?
On 6/25/07, Florian Weimer <fw@deneb.enyo.de> wrote:
> * Bu Bacoo:
>
> > What do you think about darcs?
>
> The UI is nice, but darcs is quite slow (even if you don't hit the
> exponentional corner case in the merge algorithm).
>
> My main gripe with darcs, and the prime reason why I'm moving away
> from it, is its lack of support for software archaeology. If you
> haven't tagged a tree at some point, you'll face lots of trouble when
> you try to restore something that resembles the tree you had back
> then. This is a direct consequence of the "heap of patches" approach,
> but it's a real nuisance, and the benefits of the increased
> flexibility don't make up for it, IMHO.
> -
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: DWIM ref names for push/fetch
From: Daniel Barkalow @ 2007-06-25 16:27 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vodj4o973.fsf@assigned-by-dhcp.cox.net>
On Sun, 24 Jun 2007, Junio C Hamano wrote:
> Daniel Barkalow <barkalow@iabervon.org> writes:
>
> > I was actually thinking exclusively of the matching of strings like "HEAD"
> > or "origin/next" or "master" to refs from the list of available refs. It
> > seems to me like the push code does a better job of handling the same
> > sorts of things that get_sha1() handles.
> >
> > In particular, the handling of "refs/my/funny/thing" is really wrong: it
> > gets treated as refs/heads/refs/my/funny/thing.
>
> git-parse-remote.sh::canon_refs_list_for_fetch() seems to say
> otherwise, though.
>
> - When unspecified, or explicitly spelled HEAD, take HEAD;
> - Anything that begins with refs/, use it as is;
> - Anything that begins with heads/, tags/, remotes/, assume
> it is a branch, a tag, or a tracking branch;
> - Otherwise assume a branch;
>
> So I suspect refs/my/funny/thing is covered by the second rule.
Ah, okay. I think a few bits got lost somewhat in Julian's translation to
C. I agree with the first three rules there, and with the last rule being
the last rule, and sticking more things in between those sets is easy
enough.
> But I do agree "otherwise assume a branch" part has huge room
> for improvement. Especially...
>
> > I think that "origin/next"
> > should also be assumed to be refs/remotes/origin/next instead of
> > refs/heads/origin/next, at least if you have refs/remotes/origin/ and not
> > refs/heads/origin/.
>
> ... I think that makes perfect sense -- the code should
> interpret your example as a request to start using a new
> tracking branch refs/remotes/origin/next.
Currently, it doesn't even notice if you've got the tracking branch
already. Should it have some rule to prefer things that exist over things
that don't?
When refs/remotes/origin/next doesn't exist, should it require that
refs/remotes/origin/ already exist?
In any case, the big question is whether the push code should use these
rules, too, for the corresponding portions, in which case I can share the
code (and, for that matter, the documentation, which would be even nicer,
because we've currently got a lot of hints about refspecs in different
places but nothing complete anywhere).
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* Re: [PATCH] git-svnimport: added explicit merge graph option -G
From: Stas Maximov @ 2007-06-25 16:23 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
[-- Attachment #1: Type: text/plain, Size: 6976 bytes --]
Hi Junio,
Thank you for a thorough review and good suggestions. New cumulative patch is pasted and attached.
>From: Junio C Hamano <gitster@pobox.com>
> ...
>> +# Given an SVN revision (string), returns all corresponding GIT revisions.
>> +#
>> +# Note that it is possible that one SVN revision needs to be split into two or
>> +# more GIT commits (revision). For example, this will happen if SVN user
>> +# commits two branches at once.
>> +sub svnrev_to_gitrevs($)
>> ...
>> + my $svnrev = shift;
>> + my @gitrevs;
>> + for my $b (keys(%branches)) {
>> + push (@gitrevs, $branches{$b}{$svnrev})
>> + if defined($branches{$b}{$svnrev});
>> + }
>> + return @gitrevs;
>> +}
>
>Hmph. The computation cost for this is proportional to the
>number of branches on the SVN side? Would that be a problem in
>real-life (not an objection, but am just wondering. "Not a
>problem, because..." is an acceptable answer).
I chose not to add and maintain an inverted data structure in parallel to %branches for simplicity.
Stas.
>From b17c89b82441dc3c5dc3155acee560a1d7f0149d Mon Sep 17 00:00:00 2001
From: Stas Maximov <smaximov@yahoo.com>
Date: Mon, 25 Jun 2007 09:18:35 -0700
Subject: [PATCH] git-svnimport: added explicit merge graph option -G
Allows explicit merge graph information to be provided. Each line
of merge graph file must contain a pair of SVN revision numbers
separated by space. The first number is child (merged to) SVN rev
number and the second is the parent (merged from) SVN rev number.
Comments can be started with '#' and continue to the end of line.
Empty and space-only lines are allowed and will be ignored.
Signed-off-by: Stas Maximov <smaximov@yahoo.com>
---
Documentation/git-svnimport.txt | 11 +++++-
git-svnimport.perl | 74 +++++++++++++++++++++++++++++++++++++--
2 files changed, 81 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-svnimport.txt b/Documentation/git-svnimport.txt
index e97d15e..c902b64 100644
--- a/Documentation/git-svnimport.txt
+++ b/Documentation/git-svnimport.txt
@@ -13,7 +13,8 @@ SYNOPSIS
'git-svnimport' [ -o <branch-for-HEAD> ] [ -h ] [ -v ] [ -d | -D ]
[ -C <GIT_repository> ] [ -i ] [ -u ] [-l limit_rev]
[ -b branch_subdir ] [ -T trunk_subdir ] [ -t tag_subdir ]
- [ -s start_chg ] [ -m ] [ -r ] [ -M regex ]
+ [ -s start_chg ] [ -r ]
+ [ -m ] [ -M regex ] [-G merge_graph_file ]
[ -I <ignorefile_name> ] [ -A <author_file> ]
[ -R <repack_each_revs>] [ -P <path_from_trunk> ]
<SVN_repository_URL> [ <path> ]
@@ -102,6 +103,14 @@ repository without -A.
regex. It can be used with -m to also see the default regexes.
You must escape forward slashes.
+-G <merge_graph_file>::
+ Allows explicit merge graph information to be provided. Each line
+ of merge graph file must contain a pair of SVN revision numbers
+ separated by space. The first number is child (merged to) SVN rev
+ number and the second is the parent (merged from) SVN rev number.
+ Comments can be started with '#' and continue to the end of line.
+ Empty and space-only lines are allowed and will be ignored.
+
-l <max_rev>::
Specify a maximum revision number to pull.
+
diff --git a/git-svnimport.perl b/git-svnimport.perl
index b73d649..ea0bc90 100755
--- a/git-svnimport.perl
+++ b/git-svnimport.perl
@@ -32,7 +32,7 @@ $ENV{'TZ'}="UTC";
our($opt_h,$opt_o,$opt_v,$opt_u,$opt_C,$opt_i,$opt_m,$opt_M,$opt_t,$opt_T,
$opt_b,$opt_r,$opt_I,$opt_A,$opt_s,$opt_l,$opt_d,$opt_D,$opt_S,$opt_F,
- $opt_P,$opt_R);
+ $opt_P,$opt_R,$opt_G);
sub usage() {
print STDERR <<END;
@@ -40,12 +40,13 @@ Usage: ${\basename $0} # fetch/update GIT from SVN
[-o branch-for-HEAD] [-h] [-v] [-l max_rev] [-R repack_each_revs]
[-C GIT_repository] [-t tagname] [-T trunkname] [-b branchname]
[-d|-D] [-i] [-u] [-r] [-I ignorefilename] [-s start_chg]
- [-m] [-M regex] [-A author_file] [-S] [-F] [-P project_name] [SVN_URL]
+ [-m] [-M regex] [-G merge_graph_file] [-A author_file]
+ [-S] [-F] [-P project_name] [SVN_URL]
END
exit(1);
}
-getopts("A:b:C:dDFhiI:l:mM:o:rs:t:T:SP:R:uv") or usage();
+getopts("A:b:C:dDFhiI:l:mM:G:o:rs:t:T:SP:R:uv") or usage();
usage if $opt_h;
my $tag_name = $opt_t || "tags";
@@ -80,6 +81,44 @@ if ($opt_M) {
unshift (@mergerx, qr/$opt_M/);
}
+
+# merge_graph will be used for finding all parent SVN revisions for a given SVN
+# revision. It will be implemented as a hash of arrays. Hash will be keyed with
+# the child SVN rev and contain an array of the parent SVN revisions.
+our %merge_graph;
+
+# read-in the explicit merge graph specified with -G option
+if ($opt_G) {
+ open F, '<', $opt_G
+ or die("Cannot open '$opt_G'");
+ while (<F>) {
+ chomp;
+ s/#.*//;
+ next if (/^\s*$/);
+ if (/^\s*(\d+)\s+(\d+)\s*$/) {
+ # $1: child_rev, $2: $parent_rev
+ $merge_graph{$1} ||= [];
+ push @{$merge_graph{$1}}, $2;
+ } else {
+ die "$opt_G:$. :malformed input $_\n";
+ }
+ }
+ close(F);
+}
+
+
+# Given an SVN revision (string), finds all its parent SVN revisions in the
+# merge graph.
+sub merge_graph_get_parents {
+ my $child_svnrev = shift;
+ if (exists $merge_graph{$child_svnrev}) {
+ return @{$merge_graph{$child_svnrev}};
+ }
+ return ();
+}
+
+
+
# Absolutize filename now, since we will have chdir'ed by the time we
# get around to opening it.
$opt_A = File::Spec->rel2abs($opt_A) if $opt_A;
@@ -356,6 +395,23 @@ if ($opt_A) {
open BRANCHES,">>", "$git_dir/svn2git";
+
+# Given an SVN revision (string), returns all corresponding GIT revisions.
+#
+# Note that it is possible that one SVN revision needs to be split into two or
+# more GIT commits (revision). For example, this will happen if SVN user
+# commits two branches at once.
+sub svnrev_to_gitrevs($) {
+ my $svnrev = shift;
+ my @gitrevs;
+ for my $b (keys(%branches)) {
+ push (@gitrevs, $branches{$b}{$svnrev})
+ if defined($branches{$b}{$svnrev});
+ }
+ return @gitrevs;
+}
+
+
sub node_kind($$) {
my ($svnpath, $revision) = @_;
my $pool=SVN::Pool->new;
@@ -815,6 +871,18 @@ sub commit {
}
}
}
+
+ # add parents from explicit merge graph (-G)
+ {
+ my @svnpars = merge_graph_get_parents($revision);
+ foreach my $svnp (@svnpars) {
+ my @gitpars = svnrev_to_gitrevs($svnp);
+ foreach my $gitp (@gitpars) {
+ push (@parents, $gitp);
+ }
+ }
+ }
+
my %seen_parents = ();
my @unique_parents = grep { ! $seen_parents{$_} ++ } @parents;
foreach my $bparent (@unique_parents) {
--
1.5.1.3
[-- Attachment #2: 0001-git-svnimport-added-explicit-merge-graph-option-G.patch --]
[-- Type: application/octet-stream, Size: 5760 bytes --]
From b17c89b82441dc3c5dc3155acee560a1d7f0149d Mon Sep 17 00:00:00 2001
From: Stas Maximov <smaximov@yahoo.com>
Date: Mon, 25 Jun 2007 09:18:35 -0700
Subject: [PATCH] git-svnimport: added explicit merge graph option -G
Allows explicit merge graph information to be provided. Each line
of merge graph file must contain a pair of SVN revision numbers
separated by space. The first number is child (merged to) SVN rev
number and the second is the parent (merged from) SVN rev number.
Comments can be started with '#' and continue to the end of line.
Empty and space-only lines are allowed and will be ignored.
Signed-off-by: Stas Maximov <smaximov@yahoo.com>
---
Documentation/git-svnimport.txt | 11 +++++-
git-svnimport.perl | 74 +++++++++++++++++++++++++++++++++++++--
2 files changed, 81 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-svnimport.txt b/Documentation/git-svnimport.txt
index e97d15e..c902b64 100644
--- a/Documentation/git-svnimport.txt
+++ b/Documentation/git-svnimport.txt
@@ -13,7 +13,8 @@ SYNOPSIS
'git-svnimport' [ -o <branch-for-HEAD> ] [ -h ] [ -v ] [ -d | -D ]
[ -C <GIT_repository> ] [ -i ] [ -u ] [-l limit_rev]
[ -b branch_subdir ] [ -T trunk_subdir ] [ -t tag_subdir ]
- [ -s start_chg ] [ -m ] [ -r ] [ -M regex ]
+ [ -s start_chg ] [ -r ]
+ [ -m ] [ -M regex ] [-G merge_graph_file ]
[ -I <ignorefile_name> ] [ -A <author_file> ]
[ -R <repack_each_revs>] [ -P <path_from_trunk> ]
<SVN_repository_URL> [ <path> ]
@@ -102,6 +103,14 @@ repository without -A.
regex. It can be used with -m to also see the default regexes.
You must escape forward slashes.
+-G <merge_graph_file>::
+ Allows explicit merge graph information to be provided. Each line
+ of merge graph file must contain a pair of SVN revision numbers
+ separated by space. The first number is child (merged to) SVN rev
+ number and the second is the parent (merged from) SVN rev number.
+ Comments can be started with '#' and continue to the end of line.
+ Empty and space-only lines are allowed and will be ignored.
+
-l <max_rev>::
Specify a maximum revision number to pull.
+
diff --git a/git-svnimport.perl b/git-svnimport.perl
index b73d649..ea0bc90 100755
--- a/git-svnimport.perl
+++ b/git-svnimport.perl
@@ -32,7 +32,7 @@ $ENV{'TZ'}="UTC";
our($opt_h,$opt_o,$opt_v,$opt_u,$opt_C,$opt_i,$opt_m,$opt_M,$opt_t,$opt_T,
$opt_b,$opt_r,$opt_I,$opt_A,$opt_s,$opt_l,$opt_d,$opt_D,$opt_S,$opt_F,
- $opt_P,$opt_R);
+ $opt_P,$opt_R,$opt_G);
sub usage() {
print STDERR <<END;
@@ -40,12 +40,13 @@ Usage: ${\basename $0} # fetch/update GIT from SVN
[-o branch-for-HEAD] [-h] [-v] [-l max_rev] [-R repack_each_revs]
[-C GIT_repository] [-t tagname] [-T trunkname] [-b branchname]
[-d|-D] [-i] [-u] [-r] [-I ignorefilename] [-s start_chg]
- [-m] [-M regex] [-A author_file] [-S] [-F] [-P project_name] [SVN_URL]
+ [-m] [-M regex] [-G merge_graph_file] [-A author_file]
+ [-S] [-F] [-P project_name] [SVN_URL]
END
exit(1);
}
-getopts("A:b:C:dDFhiI:l:mM:o:rs:t:T:SP:R:uv") or usage();
+getopts("A:b:C:dDFhiI:l:mM:G:o:rs:t:T:SP:R:uv") or usage();
usage if $opt_h;
my $tag_name = $opt_t || "tags";
@@ -80,6 +81,44 @@ if ($opt_M) {
unshift (@mergerx, qr/$opt_M/);
}
+
+# merge_graph will be used for finding all parent SVN revisions for a given SVN
+# revision. It will be implemented as a hash of arrays. Hash will be keyed with
+# the child SVN rev and contain an array of the parent SVN revisions.
+our %merge_graph;
+
+# read-in the explicit merge graph specified with -G option
+if ($opt_G) {
+ open F, '<', $opt_G
+ or die("Cannot open '$opt_G'");
+ while (<F>) {
+ chomp;
+ s/#.*//;
+ next if (/^\s*$/);
+ if (/^\s*(\d+)\s+(\d+)\s*$/) {
+ # $1: child_rev, $2: $parent_rev
+ $merge_graph{$1} ||= [];
+ push @{$merge_graph{$1}}, $2;
+ } else {
+ die "$opt_G:$. :malformed input $_\n";
+ }
+ }
+ close(F);
+}
+
+
+# Given an SVN revision (string), finds all its parent SVN revisions in the
+# merge graph.
+sub merge_graph_get_parents {
+ my $child_svnrev = shift;
+ if (exists $merge_graph{$child_svnrev}) {
+ return @{$merge_graph{$child_svnrev}};
+ }
+ return ();
+}
+
+
+
# Absolutize filename now, since we will have chdir'ed by the time we
# get around to opening it.
$opt_A = File::Spec->rel2abs($opt_A) if $opt_A;
@@ -356,6 +395,23 @@ if ($opt_A) {
open BRANCHES,">>", "$git_dir/svn2git";
+
+# Given an SVN revision (string), returns all corresponding GIT revisions.
+#
+# Note that it is possible that one SVN revision needs to be split into two or
+# more GIT commits (revision). For example, this will happen if SVN user
+# commits two branches at once.
+sub svnrev_to_gitrevs($) {
+ my $svnrev = shift;
+ my @gitrevs;
+ for my $b (keys(%branches)) {
+ push (@gitrevs, $branches{$b}{$svnrev})
+ if defined($branches{$b}{$svnrev});
+ }
+ return @gitrevs;
+}
+
+
sub node_kind($$) {
my ($svnpath, $revision) = @_;
my $pool=SVN::Pool->new;
@@ -815,6 +871,18 @@ sub commit {
}
}
}
+
+ # add parents from explicit merge graph (-G)
+ {
+ my @svnpars = merge_graph_get_parents($revision);
+ foreach my $svnp (@svnpars) {
+ my @gitpars = svnrev_to_gitrevs($svnp);
+ foreach my $gitp (@gitpars) {
+ push (@parents, $gitp);
+ }
+ }
+ }
+
my %seen_parents = ();
my @unique_parents = grep { ! $seen_parents{$_} ++ } @parents;
foreach my $bparent (@unique_parents) {
--
1.5.1.3
^ permalink raw reply related
* Re: [PATCH] config: add support for --bool and --int while setting values
From: Frank Lichtenheld @ 2007-06-25 16:14 UTC (permalink / raw)
To: Johannes Sixt; +Cc: git
In-Reply-To: <467FCBEA.906B14@eudaptics.com>
On Mon, Jun 25, 2007 at 04:06:34PM +0200, Johannes Sixt wrote:
> Frank Lichtenheld wrote:
> >
> > Signed-off-by: Frank Lichtenheld <frank@lichtenheld.de>
>
> Please excuse if I'm missing the big picture, but why do we need this
> change?
- Of course the user or script calling git-config can do the
normalization and error checking, if they want to. But I would
prefer to have it available in git-config.
- I would prefer that these options wouldn't be silently ignored,
because that can be confusing (at least it is documented now, but
still). So we should either using them or error out. I prefer the former.
Something that I forgot to mention in the previous mail:
One real problem with the patch is that it expands the k,m,g suffixes
for integer values. It probably shouldn't do that.
Gruesse,
--
Frank Lichtenheld <frank@lichtenheld.de>
www: http://www.djpig.de/
^ permalink raw reply
* Mark Levedahl's gitk patches
From: Johannes Sixt @ 2007-06-25 16:05 UTC (permalink / raw)
To: Paul Mackerras; +Cc: git, Mark Levedahl
Is there a chance that we get Mark Levedahl's gitk patches into 1.5.3:
gitk - Allow specifying tabstop as other than default 8 characters.
http://article.gmane.org/gmane.comp.version-control.git/47844
gitk - Update fontsize in patch / tree list
http://article.gmane.org/gmane.comp.version-control.git/47845
gitk - Make selection highlight color configurable
http://article.gmane.org/gmane.comp.version-control.git/47851
I'm mostly interested in the last one because the highlight color is
also a serious issue under MinGW.
-- Hannes
^ permalink raw reply
* Re: [PATCH 2/2] Check for IO errors after running a command
From: Linus Torvalds @ 2007-06-25 15:57 UTC (permalink / raw)
To: Jim Meyering; +Cc: Junio C Hamano, git, Matthias Lederhofer
In-Reply-To: <877ipsw2sz.fsf@rho.meyering.net>
On Mon, 25 Jun 2007, Jim Meyering wrote:
>
> That has the disadvantage of ignoring *all* pipe and socket write errors.
.. which is the only wayt to do it.
There's *no*way* to tell what the error was for the case of
if (ferror(stdout))
..
because the original errno has long long since been thrown away.
> Of course, one can probably argue that those are all unlikely.
> They may be even less likely than an actual EPIPE, but the point is
> that people and tools using git plumbing should be able to rely on
> it to report such write failures, no matter how unusual they are.
..but since what you suggest is physically impossible in a half-way
portable manner, and since all relevant such errors are likely to have
happened long before, I would suggest:
- Use my patch
OR
- Stop using stdio.
You *cannot* make stdio error handling sane. It's simply not possible.
The whole point of stdio is to simplify things, but it does so to the
point where portable and reliable error handling is not an option any
more.
Replace all command output that you care about with some stdio
replacement. We do that for all the paths we *really* care about, where
we use raw unistd IO and do our own buffering (ie things like the
"write_in_full()" stuff and "write_sha1_file()" etc.
Anybody who thinks he can handle errors with stdio is simply barking up
the wrogn tree. It can be done, but it can be done only by essentially
making stdio be a really awkward way to do IO, and you're better off with
the raw unistd.h interfaces.
In particular, you need to make sure you check *each* return value of
every single stdio operation. Even then, you don't actually know if
"errno" is set correctly if an error happens in the middle (ie most stdio
routines are just defined to return EOF or nonzero on error, and it's not
at all clear that errno is reliable).
If you don't, a buffer flush error may have happened at any time, and
whatever error code it had has been thrown away, and turned into one
single bit (the "ferror()" thing).
And yes, some libc's might have extensions that actually guarantee more.
But even then I would be *very* surprised if they actually work and have
been tested to any real degree (glibc does not. The stream error is a
single bit. I checked)
So really: if you care about a particular read or write, use the
"write_in_full()" and "read_in_full()" functions. NOTHING ELSE!
Linus
^ permalink raw reply
* Re: What's cooking in git.git (topics)
From: Jeffrey C. Ollie @ 2007-06-25 15:47 UTC (permalink / raw)
To: git
In-Reply-To: <7v645cz7vm.fsf@assigned-by-dhcp.pobox.com>
[-- Attachment #1: Type: text/plain, Size: 600 bytes --]
On Mon, 2007-06-25 at 02:43 -0700, Junio C Hamano wrote:
>
> * jo/init (Thu Jun 7 07:50:30 2007 -0500) 2 commits
> - Quiet the output from git-init when cloning, if requested.
> - Add an option to quiet git-init.
>
> I am not very much interested in this but I do not have any
> strong or otherwise feeling against it either.
It seems to me that this series is more about "DWIM" than anything. A
naïve user would expect "git clone -q" to silcence _all_ non-error
output. The output "Initialized empty Git repository in .git/" that you
get from "git init" isn't an error...
Jeff
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [PATCH] fast-import.c: detect fclose- and fflush-induced write failure
From: Jim Meyering @ 2007-06-25 14:33 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20070625141206.GC32223@spearce.org>
"Shawn O. Pearce" <spearce@spearce.org> wrote:
> Jim Meyering <jim@meyering.net> wrote:
>> There are potentially ignored write errors in fast-import.c.
> ...
>> diff --git a/fast-import.c b/fast-import.c
>> @@ -793,7 +793,9 @@ static void end_packfile(void)
>> fprintf(pack_edges, " %s", sha1_to_hex(t->sha1));
>> }
>> fputc('\n', pack_edges);
>> - fflush(pack_edges);
>> + if (fflush(pack_edges))
>> + die("failed to write pack-edges file: %s",
>> + strerror(errno));
>
> Hmm. That's a valid bug, if the disk is full we might not
> be able to flush.
Or if the disk is corrupted.
> But that linewrapping looks pretty ugly.
I agree.
Blame the 8-columns per indentation-level vs 80-col-max guideline.
Besides, the existing indentation level was already pretty deep there.
From the looks of the line just a few above that one, I wonder if the
80-col-max guideline applies to git.
>> +static int
>> +close_stream(FILE *stream)
>> +{
>> + int prev_fail = (ferror(stream) != 0);
>> + int fclose_fail = (fclose(stream) != 0);
>> +
>> + if (prev_fail || fclose_fail) {
>> + if (! fclose_fail)
>> + errno = 0;
>> + return EOF;
>> + }
>> + return 0;
>> +}
>> +
>> +static void
>> +close_wstream_or_die(FILE *stream, const char *file_name)
>> +{
>> + if (close_stream(stream)) {
>> + if (errno == 0)
>> + die ("%s: write failed: %s", file_name, strerror(errno));
>
> Don't you mean "if (errno != 0)" here? Right now you are printing
> "No Error" when there is no error and tossing the errno when there
> is an error.
Rats. You're right.
Thanks. But as I mentioned, close_wstream_or_die is probably not
code that should remain (at least not in its current form) in any case.
>> @@ -1369,7 +1396,18 @@ static void dump_marks(void)
>> }
>>
>> dump_marks_helper(f, 0, marks);
>> - fclose(f);
>> + if (close_stream(f) != 0) {
>
> What about just "if (close_stream(f)) {" ?
Sure.
I use both styles, best to be consistent.
>> + int close_errno = errno;
>> + rollback_lock_file(&mark_lock);
>> + failure |=
>> + (close_errno == 0
>> + ? error("Failed to write temporary marks file %s.lock",
>> + mark_file)
>> + : error("Failed to write temporary marks file %s.lock: %s",
>> + mark_file, strerror(close_errno)));
>
> Ugh. The ternary operator has many uses, but using it to decide
> which error() function you are going to call and have both cases
> bit-wise or a -1 into failure is not one of them. This would be
> a lot cleaner if the ternary operator wasn't abused here.
Of course. That's why I proposed to use an error-reporting
function that interpolates an errno value.
> And looking at this code I'm now wondering about the code above
> for close_stream(). If it returns EOF but doesn't supply a valid
> errno its because you tossed the errno that was available when you
> did the fclose(). So I'd actually say the close_stream() is bad;
No. You seem to be misreading close_stream.
Between the calls to ferror and fclose, errno is not useful.
It's when fclose succeeds yet ferror indicates there was
a preceding error that it sets errno = 0.
And it does that solely because there is already no
guarantee that errno is valid (read ferror's man page),
and this at least makes it so a close_stream caller
does not accidentally use an invalid errno value.
> if we have an error and we're going to explain there was an error
> we should explain what the error was.
>
>> @@ -2015,6 +2053,7 @@ static const char fast_import_usage[] =
>> int main(int argc, const char **argv)
>> {
>> int i, show_stats = 1;
>> + const char *pack_edges_file = NULL;
>
> This is only ever used in the "--export-pack-edges=" arm of the
> option parser. It should be local to that block, not to the
> entire function.
Um. No.
Oh, I see you "got it" below.
>> @@ -2052,10 +2091,13 @@ int main(int argc, const char **argv)
>> mark_file = a + 15;
>> else if (!prefixcmp(a, "--export-pack-edges=")) {
>> if (pack_edges)
>> - fclose(pack_edges);
>> - pack_edges = fopen(a + 20, "a");
>> + close_wstream_or_die(pack_edges,
>> + pack_edges_file);
>
> Oh, I see, its actually being reused to issue an error that we
> couldn't close the prior file. The more that I look at this we
> probably should just die() if we get a second arg with this option.
> What does it mean to give --export-pack-edges twice? Apparently
> under the current code it means use the last one, but I'm not sure
> that's sane.
Nor am I.
My first cut at this patch changed it so that there was only one
fclose, and everything was handled *outside* the arg-parsing loop.
But then I noticed that one could use a combination of --import-marks=...
and --export-pack-edges= (with more than one of each) to do what
I presume the original author must have thought was something useful.
IMHO, if it's appropriate to rewrite this code, that should be done
separately from detecting write failure.
Thanks for the feedback.
I'll be happy to redo the patch, once there is consensus on what
it should look like.
^ permalink raw reply
* Re: [PATCH] config: add support for --bool and --int while setting values
From: Johannes Sixt @ 2007-06-25 14:06 UTC (permalink / raw)
To: git
In-Reply-To: <1182780024442-git-send-email-frank@lichtenheld.de>
Frank Lichtenheld wrote:
>
> Signed-off-by: Frank Lichtenheld <frank@lichtenheld.de>
Please excuse if I'm missing the big picture, but why do we need this
change?
-- Hannes
^ permalink raw reply
* Re: [PATCH] fast-import.c: detect fclose- and fflush-induced write failure
From: Shawn O. Pearce @ 2007-06-25 14:12 UTC (permalink / raw)
To: Jim Meyering; +Cc: git
In-Reply-To: <87abuoxtio.fsf@rho.meyering.net>
Jim Meyering <jim@meyering.net> wrote:
> There are potentially ignored write errors in fast-import.c.
...
> diff --git a/fast-import.c b/fast-import.c
> @@ -793,7 +793,9 @@ static void end_packfile(void)
> fprintf(pack_edges, " %s", sha1_to_hex(t->sha1));
> }
> fputc('\n', pack_edges);
> - fflush(pack_edges);
> + if (fflush(pack_edges))
> + die("failed to write pack-edges file: %s",
> + strerror(errno));
Hmm. That's a valid bug, if the disk is full we might not
be able to flush. But that linewrapping looks pretty ugly.
> +static int
> +close_stream(FILE *stream)
> +{
> + int prev_fail = (ferror(stream) != 0);
> + int fclose_fail = (fclose(stream) != 0);
> +
> + if (prev_fail || fclose_fail) {
> + if (! fclose_fail)
> + errno = 0;
> + return EOF;
> + }
> + return 0;
> +}
> +
> +static void
> +close_wstream_or_die(FILE *stream, const char *file_name)
> +{
> + if (close_stream(stream)) {
> + if (errno == 0)
> + die ("%s: write failed: %s", file_name, strerror(errno));
Don't you mean "if (errno != 0)" here? Right now you are printing
"No Error" when there is no error and tossing the errno when there
is an error.
> @@ -1369,7 +1396,18 @@ static void dump_marks(void)
> }
>
> dump_marks_helper(f, 0, marks);
> - fclose(f);
> + if (close_stream(f) != 0) {
What about just "if (close_stream(f)) {" ?
> + int close_errno = errno;
> + rollback_lock_file(&mark_lock);
> + failure |=
> + (close_errno == 0
> + ? error("Failed to write temporary marks file %s.lock",
> + mark_file)
> + : error("Failed to write temporary marks file %s.lock: %s",
> + mark_file, strerror(close_errno)));
Ugh. The ternary operator has many uses, but using it to decide
which error() function you are going to call and have both cases
bit-wise or a -1 into failure is not one of them. This would be
a lot cleaner if the ternary operator wasn't abused here.
And looking at this code I'm now wondering about the code above
for close_stream(). If it returns EOF but doesn't supply a valid
errno its because you tossed the errno that was available when you
did the fclose(). So I'd actually say the close_stream() is bad;
if we have an error and we're going to explain there was an error
we should explain what the error was.
> @@ -2015,6 +2053,7 @@ static const char fast_import_usage[] =
> int main(int argc, const char **argv)
> {
> int i, show_stats = 1;
> + const char *pack_edges_file = NULL;
This is only ever used in the "--export-pack-edges=" arm of the
option parser. It should be local to that block, not to the
entire function.
> @@ -2052,10 +2091,13 @@ int main(int argc, const char **argv)
> mark_file = a + 15;
> else if (!prefixcmp(a, "--export-pack-edges=")) {
> if (pack_edges)
> - fclose(pack_edges);
> - pack_edges = fopen(a + 20, "a");
> + close_wstream_or_die(pack_edges,
> + pack_edges_file);
Oh, I see, its actually being reused to issue an error that we
couldn't close the prior file. The more that I look at this we
probably should just die() if we get a second arg with this option.
What does it mean to give --export-pack-edges twice? Apparently
under the current code it means use the last one, but I'm not sure
that's sane.
--
Shawn.
^ permalink raw reply
* [PATCH 3/3] config: Add --null/-z option for null-delimted output
From: Frank Lichtenheld @ 2007-06-25 14:03 UTC (permalink / raw)
To: Git Mailing List; +Cc: Junio C Hamano, Jakub Narebski, Frank Lichtenheld
In-Reply-To: <200706220156.01175.jnareb@gmail.com>
Use \n as delimiter between key and value and \0 as
delimiter after each key/value pair. This should be
easily parsable output.
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.de>
---
Documentation/git-config.txt | 18 +++++++++++++-----
builtin-config.c | 20 ++++++++++++++------
t/t1300-repo-config.sh | 32 ++++++++++++++++++++++++++++++++
3 files changed, 59 insertions(+), 11 deletions(-)
This time with documentation and test cases.
diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index bb6dbb0..8c09b88 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -9,17 +9,17 @@ git-config - Get and set repository or global options
SYNOPSIS
--------
[verse]
-'git-config' [--system | --global] name [value [value_regex]]
+'git-config' [--system | --global] [-z|--null] name [value [value_regex]]
'git-config' [--system | --global] --add name value
'git-config' [--system | --global] --replace-all name [value [value_regex]]
-'git-config' [--system | --global] [type] --get name [value_regex]
-'git-config' [--system | --global] [type] --get-all name [value_regex]
-'git-config' [--system | --global] [type] --get-regexp name_regex [value_regex]
+'git-config' [--system | --global] [type] [-z|--null] --get name [value_regex]
+'git-config' [--system | --global] [type] [-z|--null] --get-all name [value_regex]
+'git-config' [--system | --global] [type] [-z|--null] --get-regexp name_regex [value_regex]
'git-config' [--system | --global] --unset name [value_regex]
'git-config' [--system | --global] --unset-all name [value_regex]
'git-config' [--system | --global] --rename-section old_name new_name
'git-config' [--system | --global] --remove-section name
-'git-config' [--system | --global] -l | --list
+'git-config' [--system | --global] [-z|--null] -l | --list
DESCRIPTION
-----------
@@ -118,6 +118,14 @@ See also <<FILES>>.
in the config file will cause the value to be multiplied
by 1024, 1048576, or 1073741824 prior to output.
+-z, --null::
+ For all options that output values and/or keys, always
+ end values with with the null character (instead of a
+ newline). Use newline instead as a delimiter between
+ key and value. This allows for secure parsing of the
+ output without getting confused e.g. by values that
+ contain line breaks.
+
[[FILES]]
FILES
diff --git a/builtin-config.c b/builtin-config.c
index dbc2339..9d52ba8 100644
--- a/builtin-config.c
+++ b/builtin-config.c
@@ -2,7 +2,7 @@
#include "cache.h"
static const char git_config_set_usage[] =
-"git-config [ --global | --system ] [ --bool | --int ] [--get | --get-all | --get-regexp | --replace-all | --add | --unset | --unset-all] name [value [value_regex]] | --rename-section old_name new_name | --remove-section name | --list";
+"git-config [ --global | --system ] [ --bool | --int ] [ -z | --null ] [--get | --get-all | --get-regexp | --replace-all | --add | --unset | --unset-all] name [value [value_regex]] | --rename-section old_name new_name | --remove-section name | --list";
static char *key;
static regex_t *key_regexp;
@@ -12,14 +12,17 @@ static int use_key_regexp;
static int do_all;
static int do_not_match;
static int seen;
+static char delim = '=';
+static char key_delim = ' ';
+static char term = '\n';
static enum { T_RAW, T_INT, T_BOOL } type = T_RAW;
static int show_all_config(const char *key_, const char *value_)
{
if (value_)
- printf("%s=%s\n", key_, value_);
+ printf("%s%c%s%c", key_, delim, value_, term);
else
- printf("%s\n", key_);
+ printf("%s%c", key_, term);
return 0;
}
@@ -40,9 +43,9 @@ static int show_config(const char* key_, const char* value_)
if (show_keys) {
if (value_)
- printf("%s ", key_);
+ printf("%s%c", key_, key_delim);
else
- printf("%s", key_);
+ printf("%s",key_);
}
if (seen && !do_all)
dup_error = 1;
@@ -58,7 +61,7 @@ static int show_config(const char* key_, const char* value_)
key_, vptr);
}
else
- printf("%s\n", vptr);
+ printf("%s%c", vptr, term);
return 0;
}
@@ -159,6 +162,11 @@ int cmd_config(int argc, const char **argv, const char *prefix)
}
else if (!strcmp(argv[1], "--system"))
setenv("GIT_CONFIG", ETC_GITCONFIG, 1);
+ else if (!strcmp(argv[1], "--null") || !strcmp(argv[1], "-z")) {
+ term = '\0';
+ delim = '\n';
+ key_delim = '\n';
+ }
else if (!strcmp(argv[1], "--rename-section")) {
int ret;
if (argc != 4)
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 8b5e9fc..99d84cc 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -519,4 +519,36 @@ git config --list > result
test_expect_success 'value continued on next line' 'cmp result expect'
+cat > .git/config <<\EOF
+[section "sub=section"]
+ val1 = foo=bar
+ val2 = foo\nbar
+ val3 = \n\n
+ val4 =
+ val5
+EOF
+
+cat > expect <<\EOF
+Key: section.sub=section.val1
+Value: foo=bar
+Key: section.sub=section.val2
+Value: foo
+bar
+Key: section.sub=section.val3
+Value:
+
+
+Key: section.sub=section.val4
+Value:
+Key: section.sub=section.val5
+EOF
+
+git config --null --list | perl -0ne 'chop;($key,$value)=split(/\n/,$_,2);print "Key: $key\n";print "Value: $value\n" if defined($value)' > result
+
+test_expect_success '--null --list' 'cmp result expect'
+
+git config --null --get-regexp 'val[0-9]' | perl -0ne 'chop;($key,$value)=split(/\n/,$_,2);print "Key: $key\n";print "Value: $value\n" if defined($value)' > result
+
+test_expect_success '--null --get-regexp' 'cmp result expect'
+
test_done
--
1.5.2.1
^ permalink raw reply related
* [PATCH 2/3] config: Change output of --get-regexp for valueless keys
From: Frank Lichtenheld @ 2007-06-25 14:03 UTC (permalink / raw)
To: Git Mailing List; +Cc: Junio C Hamano, Jakub Narebski, Frank Lichtenheld
In-Reply-To: <200706220156.01175.jnareb@gmail.com>
Print no space after the name of a key without value.
Otherwise keys without values are printed exactly the
same as keys with empty values.
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.de>
---
builtin-config.c | 8 ++++++--
t/t1300-repo-config.sh | 6 ++++++
2 files changed, 12 insertions(+), 2 deletions(-)
I hope that nobody depends on this specific behaviour.
Backwards compatibilty would be a pain here, since the --null
patch would get really complicated
diff --git a/builtin-config.c b/builtin-config.c
index b2515f7..dbc2339 100644
--- a/builtin-config.c
+++ b/builtin-config.c
@@ -38,8 +38,12 @@ static int show_config(const char* key_, const char* value_)
regexec(regexp, (value_?value_:""), 0, NULL, 0)))
return 0;
- if (show_keys)
- printf("%s ", key_);
+ if (show_keys) {
+ if (value_)
+ printf("%s ", key_);
+ else
+ printf("%s", key_);
+ }
if (seen && !do_all)
dup_error = 1;
if (type == T_INT)
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 7731fa7..8b5e9fc 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -283,6 +283,12 @@ EOF
test_expect_success 'get variable with no value' \
'git-config --get novalue.variable ^$'
+echo novalue.variable > expect
+
+test_expect_success 'get-regexp variable with no value' \
+ 'git-config --get-regexp novalue > output &&
+ cmp output expect'
+
git-config > output 2>&1
test_expect_success 'no arguments, but no crash' \
--
1.5.2.1
^ permalink raw reply related
* [PATCH 1/3] config: Complete documentation of --get-regexp
From: Frank Lichtenheld @ 2007-06-25 14:03 UTC (permalink / raw)
To: Git Mailing List; +Cc: Junio C Hamano, Jakub Narebski, Frank Lichtenheld
In-Reply-To: <200706220156.01175.jnareb@gmail.com>
The asciidoc documentation of the --get-regexp option was
incomplete. Add some missing pieces:
- List the option in SYNOPSIS
- Mention that key names are printed
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.de>
---
Documentation/git-config.txt | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index f2c6717..bb6dbb0 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -14,6 +14,7 @@ SYNOPSIS
'git-config' [--system | --global] --replace-all name [value [value_regex]]
'git-config' [--system | --global] [type] --get name [value_regex]
'git-config' [--system | --global] [type] --get-all name [value_regex]
+'git-config' [--system | --global] [type] --get-regexp name_regex [value_regex]
'git-config' [--system | --global] --unset name [value_regex]
'git-config' [--system | --global] --unset-all name [value_regex]
'git-config' [--system | --global] --rename-section old_name new_name
@@ -73,6 +74,7 @@ OPTIONS
--get-regexp::
Like --get-all, but interprets the name as a regular expression.
+ Also outputs the key names.
--global::
For writing options: write to global ~/.gitconfig file rather than
--
1.5.2.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox