* [PATCH 3/4] config: support parsing config data from buffers
From: Jeff King @ 2012-01-26 7:40 UTC (permalink / raw)
To: git
In-Reply-To: <20120126073547.GA28689@sigill.intra.peff.net>
The only two ways to parse config data are from a file or
from the command-line. Because the command-line format is
totally different from the file format, they don't share any
code. Therefore, to add new sources of file-like config data,
we have to refactor git_parse_file to handle reading from
something besides stdio.
To fix this, our config_file structure now holds either a
"FILE *" pointer or a memory buffer. We intercept calls to
fgetc and ungetc and either pass them along to stdio, or
fake them with our buffer. This leaves the main parsing code
intact and easy to read.
Signed-off-by: Jeff King <peff@peff.net>
---
cache.h | 1 +
config.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++++-----
2 files changed, 54 insertions(+), 5 deletions(-)
diff --git a/cache.h b/cache.h
index 21bbb0a..a298897 100644
--- a/cache.h
+++ b/cache.h
@@ -1110,6 +1110,7 @@ extern int update_server_info(int);
typedef int (*config_fn_t)(const char *, const char *, void *);
extern int git_default_config(const char *, const char *, void *);
extern int git_config_from_file(config_fn_t fn, const char *, void *);
+extern int git_config_from_buffer(config_fn_t fn, void *, const char *, char *, unsigned long );
extern void git_config_push_parameter(const char *text);
extern int git_config_from_parameters(config_fn_t fn, void *data);
extern int git_config(config_fn_t fn, void *);
diff --git a/config.c b/config.c
index b82f749..49a3d1a 100644
--- a/config.c
+++ b/config.c
@@ -18,6 +18,9 @@ typedef struct config_file {
const char *name;
int linenr;
int eof;
+ char *buf;
+ unsigned long size;
+ unsigned long cur;
struct strbuf value;
char var[MAXNAME];
} config_file;
@@ -101,19 +104,45 @@ int git_config_from_parameters(config_fn_t fn, void *data)
return nr > 0;
}
+static int get_one_char(void)
+{
+ if (cf->f)
+ return fgetc(cf->f);
+ else if (cf->buf) {
+ if (cf->cur < cf->size)
+ return cf->buf[cf->cur++];
+ return EOF;
+ }
+
+ die("BUG: attempt to read from NULL config_file");
+}
+
+static int unget_one_char(int c)
+{
+ if (cf->f)
+ ungetc(c, cf->f);
+ else if (cf->buf) {
+ if (cf->cur == 0)
+ return EOF;
+ cf->buf[--cf->cur] = c;
+ return c;
+ }
+
+ die("BUG: attempt to ungetc NULL config_file");
+}
+
static int get_next_char(void)
{
int c;
- FILE *f;
c = '\n';
- if (cf && ((f = cf->f) != NULL)) {
- c = fgetc(f);
+ if (cf && (cf->f || cf->buf)) {
+ c = get_one_char();
if (c == '\r') {
/* DOS like systems */
- c = fgetc(f);
+ c = get_one_char();
if (c != '\n') {
- ungetc(c, f);
+ unget_one_char(c);
c = '\r';
}
}
@@ -833,6 +862,9 @@ static void config_file_push(config_file *top, const char *name)
top->name = name;
top->linenr = 1;
top->eof = 0;
+ top->buf = NULL;
+ top->size = 0;
+ top->cur = 0;
strbuf_init(&top->value, 1024);
cf = top;
}
@@ -863,6 +895,22 @@ int git_config_from_file(config_fn_t fn, const char *filename, void *data)
return ret;
}
+int git_config_from_buffer(config_fn_t fn, void *data, const char *name,
+ char *buf, unsigned long size)
+{
+ int ret;
+ config_file top;
+
+ config_file_push(&top, name);
+ top.buf = buf;
+ top.size = size;
+
+ ret = git_parse_file(fn, data);
+
+ config_file_pop(&top);
+ return ret;
+}
+
const char *git_etc_gitconfig(void)
{
static const char *system_wide;
--
1.7.9.rc2.293.gaae2
^ permalink raw reply related
* [PATCH 2/4] config: factor out config file stack management
From: Jeff King @ 2012-01-26 7:38 UTC (permalink / raw)
To: git
In-Reply-To: <20120126073547.GA28689@sigill.intra.peff.net>
Because a config callback may start parsing a new file, the
global context regarding the current config file is stored
as a stack. Currently we only need to manage that stack from
git_config_from_file. Let's factor it out to allow new
sources of config data.
Signed-off-by: Jeff King <peff@peff.net>
---
config.c | 30 +++++++++++++++++++-----------
1 files changed, 19 insertions(+), 11 deletions(-)
diff --git a/config.c b/config.c
index a6966c1..b82f749 100644
--- a/config.c
+++ b/config.c
@@ -826,6 +826,23 @@ int git_default_config(const char *var, const char *value, void *dummy)
return 0;
}
+static void config_file_push(config_file *top, const char *name)
+{
+ top->prev = cf;
+ top->f = NULL;
+ top->name = name;
+ top->linenr = 1;
+ top->eof = 0;
+ strbuf_init(&top->value, 1024);
+ cf = top;
+}
+
+static void config_file_pop(config_file *top)
+{
+ strbuf_release(&top->value);
+ cf = top->prev;
+}
+
int git_config_from_file(config_fn_t fn, const char *filename, void *data)
{
int ret;
@@ -835,21 +852,12 @@ int git_config_from_file(config_fn_t fn, const char *filename, void *data)
if (f) {
config_file top;
- /* push config-file parsing state stack */
- top.prev = cf;
+ config_file_push(&top, filename);
top.f = f;
- top.name = filename;
- top.linenr = 1;
- top.eof = 0;
- strbuf_init(&top.value, 1024);
- cf = ⊤
ret = git_parse_file(fn, data);
- /* pop config-file parsing state stack */
- strbuf_release(&top.value);
- cf = top.prev;
-
+ config_file_pop(&top);
fclose(f);
}
return ret;
--
1.7.9.rc2.293.gaae2
^ permalink raw reply related
* [PATCH 1/4] config: add include directive
From: Jeff King @ 2012-01-26 7:37 UTC (permalink / raw)
To: git
In-Reply-To: <20120126073547.GA28689@sigill.intra.peff.net>
It can be useful to split your ~/.gitconfig across multiple
files. For example, you might have a "main" file which is
used on many machines, but a small set of per-machine
tweaks. Or you may want to make some of your config public
(e.g., clever aliases) while keeping other data back (e.g.,
your name or other identifying information). Or you may want
to include a number of config options in some subset of your
repos without copying and pasting (e.g., you want to
reference them from the .git/config of participating repos).
This patch introduces an include directive for config files.
It looks like:
[include]
path = /path/to/file
This is syntactically backwards-compatible with existing git
config parsers (i.e., they will see it as another config
entry and ignore it unless you are looking up include.path).
The implementation provides a "git_config_include" callback
which wraps regular config callbacks. Callers can pass it
to git_config_from_file, and it will transparently follow
any include directives, passing all of the discovered
options to the real callback.
Include directives are turned on for regular git config
parsing (i.e., when you call git_config()), as well as for
lookups via the "git config" program. They are not turned on
in other cases, including:
1. Parsing of other config-like files, like .gitmodules.
There isn't a real need, and I'd rather be conservative
and avoid unnecessary incompatibility or confusion.
2. Writing files via "git config"; we want to treat
include.* variables as literal items to be copied (or
modified), and not expand them. So "git config
--unset-all foo.bar" would operate _only_ on
.git/config, not any of its included files (just as it
also does not operate on ~/.gitconfig).
Signed-off-by: Jeff King <peff@peff.net>
---
Documentation/config.txt | 15 ++++++
Documentation/git-config.txt | 5 ++
builtin/config.c | 29 +++++++++---
cache.h | 6 +++
config.c | 58 +++++++++++++++++++++++++
t/t1305-config-include.sh | 98 ++++++++++++++++++++++++++++++++++++++++++
6 files changed, 204 insertions(+), 7 deletions(-)
create mode 100755 t/t1305-config-include.sh
diff --git a/Documentation/config.txt b/Documentation/config.txt
index abeb82b..e55dae1 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -84,6 +84,17 @@ customary UNIX fashion.
Some variables may require a special value format.
+Includes
+~~~~~~~~
+
+You can include one config file from another by setting the special
+`include.path` variable to the name of the file to be included. The
+included file is expanded immediately, as if its contents had been
+found at the location of the include directive. If the value of the
+`include.path` variable is a relative path, the path is considered to be
+relative to the configuration file in which the include directive was
+found. See below for examples.
+
Example
~~~~~~~
@@ -106,6 +117,10 @@ Example
gitProxy="ssh" for "kernel.org"
gitProxy=default-proxy ; for the rest
+ [include]
+ path = /path/to/foo.inc ; include by absolute path
+ path = foo ; expand "foo" relative to the current file
+
Variables
~~~~~~~~~
diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index e7ecf5d..aa8303b 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -178,6 +178,11 @@ See also <<FILES>>.
Opens an editor to modify the specified config file; either
'--system', '--global', or repository (default).
+--includes::
+--no-includes::
+ Respect `include.*` directives in config files when looking up
+ values. Defaults to on.
+
[[FILES]]
FILES
-----
diff --git a/builtin/config.c b/builtin/config.c
index d35c06a..9105f87 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -25,6 +25,7 @@ static const char *given_config_file;
static int actions, types;
static const char *get_color_slot, *get_colorbool_slot;
static int end_null;
+static int respect_includes = 1;
#define ACTION_GET (1<<0)
#define ACTION_GET_ALL (1<<1)
@@ -74,6 +75,8 @@ static struct option builtin_config_options[] = {
OPT_BIT(0, "path", &types, "value is a path (file or directory name)", TYPE_PATH),
OPT_GROUP("Other"),
OPT_BOOLEAN('z', "null", &end_null, "terminate values with NUL byte"),
+ OPT_BOOLEAN(0, "includes", &respect_includes,
+ "respect include directives on lookup"),
OPT_END(),
};
@@ -161,6 +164,9 @@ static int get_value(const char *key_, const char *regex_)
int ret = -1;
char *global = NULL, *repo_config = NULL;
const char *system_wide = NULL, *local;
+ struct git_config_include_data inc;
+ config_fn_t fn;
+ void *data;
local = config_exclusive_filename;
if (!local) {
@@ -213,19 +219,28 @@ static int get_value(const char *key_, const char *regex_)
}
}
+ fn = show_config;
+ data = NULL;
+ if (respect_includes) {
+ inc.fn = fn;
+ inc.data = data;
+ fn = git_config_include;
+ data = &inc;
+ }
+
if (do_all && system_wide)
- git_config_from_file(show_config, system_wide, NULL);
+ git_config_from_file(fn, system_wide, data);
if (do_all && global)
- git_config_from_file(show_config, global, NULL);
+ git_config_from_file(fn, global, data);
if (do_all)
- git_config_from_file(show_config, local, NULL);
- git_config_from_parameters(show_config, NULL);
+ git_config_from_file(fn, local, data);
+ git_config_from_parameters(fn, data);
if (!do_all && !seen)
- git_config_from_file(show_config, local, NULL);
+ git_config_from_file(fn, local, data);
if (!do_all && !seen && global)
- git_config_from_file(show_config, global, NULL);
+ git_config_from_file(fn, global, data);
if (!do_all && !seen && system_wide)
- git_config_from_file(show_config, system_wide, NULL);
+ git_config_from_file(fn, system_wide, data);
free(key);
if (regexp) {
diff --git a/cache.h b/cache.h
index 10afd71..21bbb0a 100644
--- a/cache.h
+++ b/cache.h
@@ -1138,6 +1138,12 @@ extern const char *get_commit_output_encoding(void);
extern int git_config_parse_parameter(const char *, config_fn_t fn, void *data);
+struct git_config_include_data {
+ config_fn_t fn;
+ void *data;
+};
+int git_config_include(const char *name, const char *value, void *vdata);
+
extern const char *config_exclusive_filename;
#define MAX_GITNAME (1000)
diff --git a/config.c b/config.c
index 40f9c6d..a6966c1 100644
--- a/config.c
+++ b/config.c
@@ -874,10 +874,68 @@ int git_config_system(void)
return !git_env_bool("GIT_CONFIG_NOSYSTEM", 0);
}
+static int handle_path_include(const char *path, void *data)
+{
+ int ret = 0;
+ struct strbuf buf = STRBUF_INIT;
+
+ /*
+ * Use an absolute value as-is, but interpret relative paths
+ * based on the including config file.
+ */
+ if (!is_absolute_path(path)) {
+ char *slash;
+ if (!cf)
+ return error("relative config includes must come from files");
+ strbuf_addstr(&buf, absolute_path(cf->name));
+ slash = find_last_dir_sep(buf.buf);
+ if (!slash)
+ die("BUG: no directory separator in an absolute path?");
+ strbuf_setlen(&buf, slash - buf.buf + 1);
+ strbuf_addf(&buf, "%s", path);
+ path = buf.buf;
+ }
+
+ if (!access(path, R_OK))
+ ret = git_config_from_file(git_config_include, path, data);
+ strbuf_release(&buf);
+ return ret;
+}
+
+int git_config_include(const char *name, const char *value, void *vdata)
+{
+ const struct git_config_include_data *data = vdata;
+ const char *type;
+ int ret;
+
+ /*
+ * Pass along all values, including "include" directives; this makes it
+ * possible to query information on the includes themselves.
+ */
+ ret = data->fn(name, value, data->data);
+ if (ret < 0)
+ return ret;
+
+ if (prefixcmp(name, "include."))
+ return ret;
+ type = strrchr(name, '.') + 1;
+
+ if (!strcmp(type, "path"))
+ ret = handle_path_include(value, vdata);
+
+ return ret;
+}
+
int git_config_early(config_fn_t fn, void *data, const char *repo_config)
{
int ret = 0, found = 0;
const char *home = NULL;
+ struct git_config_include_data inc;
+
+ inc.fn = fn;
+ inc.data = data;
+ fn = git_config_include;
+ data = &inc;
/* Setting $GIT_CONFIG makes git read _only_ the given config file. */
if (config_exclusive_filename)
diff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh
new file mode 100755
index 0000000..4db3091
--- /dev/null
+++ b/t/t1305-config-include.sh
@@ -0,0 +1,98 @@
+#!/bin/sh
+
+test_description='test config file include directives'
+. ./test-lib.sh
+
+test_expect_success 'include file by absolute path' '
+ echo "[test]one = 1" >one &&
+ echo "[include]path = \"$PWD/one\"" >base &&
+ echo 1 >expect &&
+ git config -f base test.one >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'include file by relative path' '
+ echo "[test]one = 1" >one &&
+ echo "[include]path = one" >base &&
+ echo 1 >expect &&
+ git config -f base test.one >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'recursive relative paths' '
+ mkdir subdir &&
+ echo "[test]three = 3" >subdir/three &&
+ echo "[include]path = three" >subdir/two &&
+ echo "[include]path = subdir/two" >base &&
+ echo 3 >expect &&
+ git config -f base test.three >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'include options can still be examined' '
+ echo "[test]one = 1" >one &&
+ echo "[include]path = one" >base &&
+ echo one >expect &&
+ git config -f base include.path >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'listing includes option and expansion' '
+ echo "[test]one = 1" >one &&
+ echo "[include]path = one" >base &&
+ cat >expect <<-\EOF &&
+ include.path=one
+ test.one=1
+ EOF
+ git config -f base --list >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'writing config file does not expand includes' '
+ echo "[test]one = 1" >one &&
+ echo "[include]path = one" >base &&
+ git config -f base test.two 2 &&
+ echo 2 >expect &&
+ git config -f base --no-includes test.two >actual &&
+ test_cmp expect actual &&
+ test_must_fail git config -f base --no-includes test.one
+'
+
+test_expect_success 'config modification does not affect includes' '
+ echo "[test]one = 1" >one &&
+ echo "[include]path = one" >base &&
+ git config -f base test.one 2 &&
+ echo 1 >expect &&
+ git config -f one test.one >actual &&
+ test_cmp expect actual &&
+ cat >expect <<-\EOF &&
+ 1
+ 2
+ EOF
+ git config -f base --get-all test.one >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'missing include files are ignored' '
+ cat >base <<-\EOF &&
+ [include]path = foo
+ [test]value = yes
+ EOF
+ echo yes >expect &&
+ git config -f base test.value >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'absolute includes from command line work' '
+ echo "[test]one = 1" >one &&
+ echo 1 >expect &&
+ git -c include.path="$PWD/one" config test.one >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'relative includes from command line fail' '
+ echo "[test]one = 1" >one &&
+ test_must_fail git -c include.path=one config test.one
+'
+
+test_done
--
1.7.9.rc2.293.gaae2
^ permalink raw reply related
* [RFC/PATCH 0/4] config include directives
From: Jeff King @ 2012-01-26 7:35 UTC (permalink / raw)
To: git
This series provides a way for config files to include other config
files in two ways:
1. From other files in the filesystem. This is implemented by patch 1
below, and is hopefully straightforward and uncontroversial. See
that patch for more rationale.
2. From blobs in the repo. This is implemented by patch 4, with
patches 2 and 3 providing the necessary refactoring. This
is one way of implementing the often asked-for "respect shared
config inside the repo" feature, but attempts to mitigate some of
the security concerns. The interface for using it safely is a bit
raw, but I think it's a sane building block, and somebody could
write a fancier shared-config updater on top of it if they wanted
to.
[1/4]: config: add include directive
[2/4]: config: factor out config file stack management
[3/4]: config: support parsing config data from buffers
[4/4]: config: allow including config from repository blobs
-Peff
^ permalink raw reply
* Re: [PATCH v3 0/3] git-p4: Search for parent commit on branch creation
From: Pete Wyckoff @ 2012-01-26 4:21 UTC (permalink / raw)
To: Vitor Antunes; +Cc: git, Luke Diamand
In-Reply-To: <1327535304-11332-1-git-send-email-vitor.hda@gmail.com>
vitor.hda@gmail.com wrote on Wed, 25 Jan 2012 23:48 +0000:
> I think this will, hopefully, be the final version of this series of
> patches. This version includes the following changes since v2:
>
> - Move search algorithm into its own function.
> - Use lists instead of strings on shell commands.
> - Some small (almost cosmetic) updates to test cases.
Whole series
Acked-by: Pete Wyckoff <pw@padd.com>
Thanks for making all the changes.
> Pete Wyckoff (1):
> git-p4: Change p4 command invocation
>
> Vitor Antunes (2):
> git-p4: Search for parent commit on branch creation
> git-p4: Add test case for complex branch import
>
> contrib/fast-import/git-p4 | 48 +++++++++++++++++++++-
> t/t9801-git-p4-branch.sh | 94 ++++++++++++++++++++++++++++++++++++++++---
> 2 files changed, 133 insertions(+), 9 deletions(-)
>
> --
> 1.7.8.3
>
^ permalink raw reply
* Re: Question about commit message wrapping
From: Sidney San Martín @ 2012-01-26 1:50 UTC (permalink / raw)
To: Drew Northup
Cc: Michael Haggerty, Frans Klaver, Holger Hellmuth, Andrew Ardill,
JakubNarebski, git@vger.kernel.org
In-Reply-To: <1325433812.4752.88.camel@drew-northup.unet.maine.edu>
Hey Drew. Sorry about the lag; I really appreciate your (and the other guys on the list) taking the time to write back. You made some interesting points in your last email and I want to respond to them.
- - -
It may come down to personal preference and how we are each used to editing text.
All the programs I use on a daily basis use soft line wrapping (not encoding any of the line breaks I see on screen into what I’m typing). Most of them use proportional type too. So, I guess I don’t *expect* to be able to manage how text wraps. Neither my email client nor the editor I use to write commit messages are set up for hard wrapping.
I posed the question to the list because having to switch into “I have to hit return every so often” mode (or play with my editor) to write commit message screws with my head, and lately I just stopped doing it in my own projects. I was wondering if anyone else was thinking along the same lines and would prefer to write their commits without line breaks (except to break paragraphs, lists, etc.).
On Jan 1, 2012, at 8:03 AM, Drew Northup wrote:
> FWIW, I'm leaving text in this email as my mail client found it (and not
> reflowing as I usually do). You can clearly see the effect of one mail
> client assuming that the end display is doing all of the wrapping (I'll
> not name names). When I first read the mail it looked fine because my
> mail client IGNORED the inconsistencies in line wrap modes.
That's interesting — it looks like my main mail client, Apple Mail, uses tricks (which I think we've discovered are mostly powered by quoted-printable encoding) to get around the old SMTP line length limits. (Is there where the differences in behavior between mail clients come from? I don't know much about the history.)
As a result, I’m not really sure what you mean. What effects and inconsistencies are you seeing? My phone, my computer, and my webmail (Google) all show pretty much the same thing: the lines from your and Michael’s emails are wrapped at ~72 characters, and the lines from my email are wrapped to the width of the window. But, they all look OK.
The only side effect is that on my phone, your/Michael’s text breaks about half way through every other line (since it's wrapped to a wider width than fits on the screen).
I don’t think I have ever manually reflowed text like you mention. When do you do it, and why?
(It may be that the thing I’m proposing wouldn’t be appropriate for git itself and other repos that use email to exchange patches. I use remotes for the repos I use on a daily basis, so I don’t have to deal with email clients like you guys do.)
> In virtually all modern tools the default font is the "system default"
> font, which is typically variable width. In some places I've even seen
> variable pitch font rendering (I know there's a more technical term for
> it, but I'm not taking the time to look it up right now) used, which is
> distinct in that it makes the text easier to read when there are
> potentially overlapping descenders and ascenders on adjoining lines
> while leaving text without that feature unchanged in line spacing and
> kerning. Try rendering ASCII-ART with that enabled!
> However, it is a rare GUI tool that does not allow the user to change
> the font to something more appropriate (I use fixed-width fonts for most
> programming and scripting, but they are not any more helpful for reading
> log messages for instance).
That’s not really the motivation behind my proposal, but I checked and actually none of the GUIs I tested before have options to change to a fixed-width font for commit messages.
It’s not really the point, though. Proportional type is nice! Have you ever seen Docco? It’s a tool that takes a well-commented source file like <http://documentcloud.github.com/backbone/backbone.js> and creates a beautiful two-column view like <http://documentcloud.github.com/backbone/docs/backbone.html> with the comments on one side in a nice (for text) proportional font and the code on the other in a nice (for code) monospaced one. The text and the code both each look great. I want the guys who develop the next generation of git clients to have the same freedom to make commit messages look great.
ASCII art in text has been a casualty of the rise of proportional type… but there are ways to save it (GitHub’s code blocks are an example); teaching everyone (and forcing the tools) to format text the old way is, I believe, the wrong approach.
> Text-based programming tools usually just
> use the console font, whatever it is--and woe be to the programmer who
> switches their "console" font to something variable width. (Doing so
> makes any application written with curses/ncurses in mind look very very
> odd as well.)
What about manpages? They use the console font but lines get wrapped to the width of the terminal (and I want this feature of git to do that, too). They’re *not* preformatted in terms of width.
>> - don’t insert line breaks when you write a commit message (and don't provide a way to do so automatically),
>
> Most of the "tools" I have seen that ignore all user-entered line breaks
> are actually poorly written applications attempting to protect some sort
> of backing database from an injection attack. Given that, many WIKI
> systems typically ignore single line breaks when rendering (double line
> breaks are taken to be paragraph breaks in those cases I am aware of),
> so any argument about that quickly becomes moot as well. If somebody is
> writing a tool that does not allow me to force multiple line breaks when
> desired then so far as I am concerned their tool is broken. I don't see
> a point in changing GIT as a whole because somebody writes a broken GUI
> implementation.
Misunderstanding — the tools I tested don’t insert don’t insert line breaks for you to help you wrap your commit messages to a certain width. They keep user-entered line breaks (but since they use proportional type and lack column counters, there’s no way to wrap to 72 columns, even by hand, except by copying and pasting from another editor… or counting).
>
>> - do wrap commit messages when showing them.
>>
>> Jakub, you said that education was the answer to getting some consistency in line wrapping, but I have trouble imagining the makers of new tools using fixed-width text for anything other than code.
>
> Remember, as soon as you think you've idiot-proofed something somebody
> builds a better idiot. That's why Jakub (and many others of us) would
> prefer just to tell people about the way things are intended to work and
> then get out of the way and let people make their own mistakes.
See above, this isn’t idiot-proofing. New tools use proportional fonts because they’re more readable, and soft wrapping because it makes sense in the context that they create and read commit messages.
>>> And given that commit messages sometimes
>>> contain "flowable" paragraph text, sometimes code snippets, sometimes
>>> ASCII art, etc, no automatic wrapping will work correctly unless
>>> everybody agrees that commit messages must be written in some specific
>>> form of markup (or lightweight markup). And I can't imagine such a
>>> thing ever happening.
>>
>> The two biggest websites I know of for talking about code, GitHub and Stack Overflow, both adopted flavors of Markdown. It is basically the formatting syntax already used for commit messages in the Git project itself (this email too), so can be formatted to look good in a specific environment (i.e. proportional fonts) and looks good by itself.
>>
>> (Actually, as far as I can tell commit messages are the only place GitHub doesn’t currently render user-entered text as Markdown.)
>>
>> I think, now and in the future, consistency will be found most easily in in Markdown-like formatting and least in 80 columns of fixed-width text.
>
> Given that there is little consensus even between Markdown-like
> formatting methods (which have in some cases been around since the
> advent of movable type presses, so far as I am aware) I have to agree
> with Michael here.
I think that the syntax laid out at <http://daringfireball.net/projects/markdown/syntax> is pretty consistently-used. Any differences that you think would affect this case? I think only a small subset would be relevant (paragraph breaks, ordered and unordered lists, and indented code blocks) and every implementation I’ve seen handles those the same way.
I think that supporting this small subset, and only for users who turn it on, would work. It matches how many commits are already formatted, there’s nothing new to learn.
P.S. Happy belated New Year!
^ permalink raw reply
* [PATCH v3 2/3] git-p4: Add test case for complex branch import
From: Vitor Antunes @ 2012-01-25 23:48 UTC (permalink / raw)
To: git; +Cc: Pete Wyckoff, Luke Diamand, Vitor Antunes
In-Reply-To: <1327535304-11332-1-git-send-email-vitor.hda@gmail.com>
Check if branches created from old changelists are correctly imported.
Also included some updates to simple branch test so that both are
coherent in respect to each other.
Signed-off-by: Vitor Antunes <vitor.hda@gmail.com>
---
t/t9801-git-p4-branch.sh | 94 ++++++++++++++++++++++++++++++++++++++++++---
1 files changed, 87 insertions(+), 7 deletions(-)
diff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh
index a25f18d..6ff713b 100755
--- a/t/t9801-git-p4-branch.sh
+++ b/t/t9801-git-p4-branch.sh
@@ -172,9 +172,9 @@ test_expect_success 'add simple p4 branches' '
echo file1 >file1 &&
echo file2 >file2 &&
p4 add file1 file2 &&
- p4 submit -d "branch1" &&
+ p4 submit -d "Create branch1" &&
p4 integrate //depot/branch1/... //depot/branch2/... &&
- p4 submit -d "branch2" &&
+ p4 submit -d "Integrate branch2 from branch1" &&
echo file3 >file3 &&
p4 add file3 &&
p4 submit -d "add file3 in branch1" &&
@@ -182,7 +182,7 @@ test_expect_success 'add simple p4 branches' '
echo update >>file2 &&
p4 submit -d "update file2 in branch1" &&
p4 integrate //depot/branch1/... //depot/branch3/... &&
- p4 submit -d "branch3"
+ p4 submit -d "Integrate branch3 from branch1"
)
'
@@ -203,17 +203,17 @@ test_expect_success 'git-p4 clone simple branches' '
test -f file1 &&
test -f file2 &&
test -f file3 &&
- grep -q update file2 &&
+ grep update file2 &&
git reset --hard p4/depot/branch2 &&
test -f file1 &&
test -f file2 &&
test ! -f file3 &&
- test_must_fail grep -q update file2 &&
+ test_must_fail grep update file2 &&
git reset --hard p4/depot/branch3 &&
test -f file1 &&
test -f file2 &&
test -f file3 &&
- grep -q update file2 &&
+ grep update file2 &&
cd "$cli" &&
cd branch1 &&
p4 edit file2 &&
@@ -222,7 +222,87 @@ test_expect_success 'git-p4 clone simple branches' '
cd "$git" &&
git reset --hard p4/depot/branch1 &&
"$GITP4" rebase &&
- grep -q file2_ file2
+ grep file2_ file2
+ )
+'
+
+# Create a complex branch structure in P4 depot to check if they are correctly
+# cloned. The branches are created from older changelists to check if git-p4 is
+# able to correctly detect them.
+# The final expected structure is:
+# `branch1
+# | `- file1
+# | `- file2 (updated)
+# | `- file3
+# `branch2
+# | `- file1
+# | `- file2
+# `branch3
+# | `- file1
+# | `- file2 (updated)
+# | `- file3
+# `branch4
+# | `- file1
+# | `- file2
+# `branch5
+# `- file1
+# `- file2
+# `- file3
+test_expect_success 'git-p4 add complex branches' '
+ test_when_finished cleanup_git &&
+ test_create_repo "$git" &&
+ (
+ cd "$cli" &&
+ changelist=$(p4 changes -m1 //depot/... | cut -d" " -f2) &&
+ changelist=$(($changelist - 5)) &&
+ p4 integrate //depot/branch1/...@$changelist //depot/branch4/... &&
+ p4 submit -d "Integrate branch4 from branch1@${changelist}" &&
+ changelist=$(($changelist + 2)) &&
+ p4 integrate //depot/branch1/...@$changelist //depot/branch5/... &&
+ p4 submit -d "Integrate branch5 from branch1@${changelist}"
+ )
+'
+
+# Configure branches through git-config and clone them. git-p4 will only be able
+# to clone the original structure if it is able to detect the origin changelist
+# of each branch.
+test_expect_success 'git-p4 clone complex branches' '
+ test_when_finished cleanup_git &&
+ test_create_repo "$git" &&
+ (
+ cd "$git" &&
+ git config git-p4.branchList branch1:branch2 &&
+ git config --add git-p4.branchList branch1:branch3 &&
+ git config --add git-p4.branchList branch1:branch4 &&
+ git config --add git-p4.branchList branch1:branch5 &&
+ "$GITP4" clone --dest=. --detect-branches //depot@all &&
+ git log --all --graph --decorate --stat &&
+ git reset --hard p4/depot/branch1 &&
+ test_path_is_file file1 &&
+ test_path_is_file file2 &&
+ test_path_is_file file3 &&
+ grep update file2 &&
+ git reset --hard p4/depot/branch2 &&
+ test_path_is_file file1 &&
+ test_path_is_file file2 &&
+ test_path_is_missing file3 &&
+ test_must_fail grep update file2 &&
+ git reset --hard p4/depot/branch3 &&
+ test_path_is_file file1 &&
+ test_path_is_file file2 &&
+ test_path_is_file file3 &&
+ grep update file2 &&
+ git reset --hard p4/depot/branch4 &&
+ test_path_is_file file1 &&
+ test_path_is_file file2 &&
+ test_path_is_missing file3 &&
+ test_must_fail grep update file2 &&
+ git reset --hard p4/depot/branch5 &&
+ test_path_is_file file1 &&
+ test_path_is_file file2 &&
+ test_path_is_file file3 &&
+ test_must_fail grep update file2 &&
+ test_path_is_missing .git/git-p4-tmp
)
'
--
1.7.8.3
^ permalink raw reply related
* [PATCH v3 1/3] git-p4: Search for parent commit on branch creation
From: Vitor Antunes @ 2012-01-25 23:48 UTC (permalink / raw)
To: git; +Cc: Pete Wyckoff, Luke Diamand, Vitor Antunes
In-Reply-To: <1327535304-11332-1-git-send-email-vitor.hda@gmail.com>
To find out which is its parent the commit of the new branch is compared
sequentially to each blob of the parent branch from the newest to the
oldest. The first blob which results in a zero diff is considered the
parent commit. If none is found, then the commit is applied to the top
of the parent branch.
A fast-import "checkpoint" call is required because diff-tree is only
able to work with blobs on disk. But most of these commits will not be
part of the final imported tree, making fast-import fail. To avoid this,
the temporary branches are tracked and then removed at the end of the
import process.
Signed-off-by: Vitor Antunes <vitor.hda@gmail.com>
---
contrib/fast-import/git-p4 | 46 +++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 45 insertions(+), 1 deletions(-)
diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index 3e1aa27..584cc41 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -1429,6 +1429,8 @@ class P4Sync(Command, P4UserMap):
self.cloneExclude = []
self.useClientSpec = False
self.clientSpecDirs = None
+ self.tempBranches = []
+ self.tempBranchLocation = "git-p4-tmp"
if gitConfig("git-p4.syncFromOrigin") == "false":
self.syncWithOrigin = False
@@ -1450,6 +1452,14 @@ class P4Sync(Command, P4UserMap):
.replace("%25", "%")
return path
+ # Force a checkpoint in fast-import and wait for it to finish
+ def checkpoint(self):
+ self.gitStream.write("checkpoint\n\n")
+ self.gitStream.write("progress checkpoint\n\n")
+ out = self.gitOutput.readline()
+ if self.verbose:
+ print "checkpoint finished: " + out
+
def extractFilesFromCommit(self, commit):
self.cloneExclude = [re.sub(r"\.\.\.$", "", path)
for path in self.cloneExclude]
@@ -1948,6 +1958,20 @@ class P4Sync(Command, P4UserMap):
self.importChanges(changes)
return True
+ def searchParent(self, parent, branch, target):
+ parentFound = False
+ for blob in read_pipe_lines(["git", "rev-list", "--reverse", "--no-merges", parent]):
+ blob = blob.strip()
+ if len(read_pipe(["git", "diff-tree", blob, target])) == 0:
+ parentFound = True
+ if self.verbose:
+ print "Found parent of %s in commit %s" % (branch, blob)
+ break
+ if parentFound:
+ return blob
+ else:
+ return None
+
def importChanges(self, changes):
cnt = 1
for change in changes:
@@ -2004,7 +2028,21 @@ class P4Sync(Command, P4UserMap):
parent = self.initialParents[branch]
del self.initialParents[branch]
- self.commit(description, filesForCommit, branch, [branchPrefix], parent)
+ blob = None
+ if len(parent) > 0:
+ tempBranch = os.path.join(self.tempBranchLocation, "%d" % (change))
+ if self.verbose:
+ print "Creating temporary branch: " + tempBranch
+ self.commit(description, filesForCommit, tempBranch, [branchPrefix])
+ self.tempBranches.append(tempBranch)
+ self.checkpoint()
+ blob = self.searchParent(parent, branch, tempBranch)
+ if blob:
+ self.commit(description, filesForCommit, branch, [branchPrefix], blob)
+ else:
+ if self.verbose:
+ print "Parent of %s not found. Committing into head of %s" % (branch, parent)
+ self.commit(description, filesForCommit, branch, [branchPrefix], parent)
else:
files = self.extractFilesFromCommit(description)
self.commit(description, files, self.branch, self.depotPaths,
@@ -2339,6 +2377,12 @@ class P4Sync(Command, P4UserMap):
self.gitOutput.close()
self.gitError.close()
+ # Cleanup temporary branches created during import
+ if self.tempBranches != []:
+ for branch in self.tempBranches:
+ read_pipe("git update-ref -d %s" % branch)
+ os.rmdir(os.path.join(os.environ.get("GIT_DIR", ".git"), self.tempBranchLocation))
+
return True
class P4Rebase(Command):
--
1.7.8.3
^ permalink raw reply related
* [PATCH v3 3/3] git-p4: Change p4 command invocation
From: Vitor Antunes @ 2012-01-25 23:48 UTC (permalink / raw)
To: git; +Cc: Pete Wyckoff, Luke Diamand, Vitor Antunes
In-Reply-To: <1327535304-11332-1-git-send-email-vitor.hda@gmail.com>
From: Pete Wyckoff <pw@padd.com>
Change p4 command invocation to avoid going through the shell. This
allows names with spaces and wildcards to work.
Signed-off-by: Pete Wyckoff <pw@padd.com>
Signed-off-by: Vitor Antunes <vitor.hda@gmail.com>
---
contrib/fast-import/git-p4 | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index 584cc41..74d3613 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -1975,7 +1975,7 @@ class P4Sync(Command, P4UserMap):
def importChanges(self, changes):
cnt = 1
for change in changes:
- description = p4Cmd("describe %s" % change)
+ description = p4Cmd(["describe", str(change)])
self.updateOptionDict(description)
if not self.silent:
--
1.7.8.3
^ permalink raw reply related
* [PATCH v3 0/3] git-p4: Search for parent commit on branch creation
From: Vitor Antunes @ 2012-01-25 23:48 UTC (permalink / raw)
To: git; +Cc: Pete Wyckoff, Luke Diamand, Vitor Antunes
I think this will, hopefully, be the final version of this series of
patches. This version includes the following changes since v2:
- Move search algorithm into its own function.
- Use lists instead of strings on shell commands.
- Some small (almost cosmetic) updates to test cases.
Pete Wyckoff (1):
git-p4: Change p4 command invocation
Vitor Antunes (2):
git-p4: Search for parent commit on branch creation
git-p4: Add test case for complex branch import
contrib/fast-import/git-p4 | 48 +++++++++++++++++++++-
t/t9801-git-p4-branch.sh | 94 ++++++++++++++++++++++++++++++++++++++++---
2 files changed, 133 insertions(+), 9 deletions(-)
--
1.7.8.3
^ permalink raw reply
* Re: [PATCH 5/5] run-command: Error out if interpreter not found
From: Frans Klaver @ 2012-01-25 23:09 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Jonathan Nieder, git, Junio C. Hamano
In-Reply-To: <4F205028.4060606@kdbg.org>
On Wed, 25 Jan 2012 19:55:36 +0100, Johannes Sixt <j6t@kdbg.org> wrote:
>> I guess it could use similar code to this patch series to tackle
>> all this.
>
> No thanks. IMHO, this is already too much code for too little gain.
That's OK. If it's not only you who feels this way, I'd better spend my
time on something that does add value.
Thanks for having a look in any case.
Frans
^ permalink raw reply
* Re: [PATCH 3/5] run-command: Elaborate execvp error checking
From: Frans Klaver @ 2012-01-25 22:59 UTC (permalink / raw)
To: Johannes Sixt; +Cc: git, Junio C. Hamano, Jonathan Nieder
In-Reply-To: <4F205212.5080007@kdbg.org>
On Wed, 25 Jan 2012 20:03:46 +0100, Johannes Sixt <j6t@kdbg.org> wrote:
> Am 24.01.2012 23:32, schrieb Frans Klaver:
>> +static void inspect_failure(const char *argv0, int silent_exec_failure)
>> +{
>> + int err = errno;
>> + struct strbuf sb = STRBUF_INIT;
>> +
>> + /* errors not related to path */
>> + if (errno == E2BIG || errno == ENOMEM)
>> + die_file_error(argv0, err);
>> +
>> + if (strchr(argv0, '/')) {
>> + if (file_exists(argv0)) {
>> + strbuf_add(&sb, argv0, strlen(argv0));
>> + inspect_file(&sb, err, argv0);
>
> Can we end up here if errno == ENOENT? If so, silent_exec_failure must
> be checked. (inspect_file does not return.)
Hm, good catch. Yes, we can if the interpreter isn't found. I never
intended to actually leave the "interpreter not found" ENOENT case in it's
current shape, so it probably slipped through. Will fix inspect_failure
here to guarantee silent_exec_failure is heeded. Patch 5/5 would probably
remove it again.
Thanks.
^ permalink raw reply
* Re: [PATCH 3/5] run-command: Elaborate execvp error checking
From: Frans Klaver @ 2012-01-25 22:48 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, Junio C. Hamano, Johannes Sixt
In-Reply-To: <20120125192222.GH1824@burratino>
On Wed, 25 Jan 2012 20:22:22 +0100, Jonathan Nieder <jrnieder@gmail.com>
wrote:
> Frans Klaver wrote:
>> Jonathan Nieder wrote:
>
>>> Could you give an example?
>>
>> The case that triggered me to work on this. I had an incorrect entry
>> in my PATH and some aliasing tests failed. The generated command
>> output was something like
>>
>> fatal: script: Access Denied
>
> Sorry for the lack of clarity. I meant that a (precise) "before and
> after" example could make the commit message a lot easier to
> understand.
Ah I see. I'll add something along those lines.
> [...]
>>> What happens on Windows?
>>
>> I haven't changed anything on the windows side, so that probably
>> sticks to the old behavior.
>
> This was mostly a comment on the change description --- unless I look
> at the patch, if I try this out on Windows after reading the changelog
> I would end up utterly confused. For patch 5/5, it also brings up
> worries about consistency --- if systems are going to be relying on a
> missing #! interpreter being treated differently from a missing script
> for the sake of silent_exec_failure, do the same considerations apply
> on Windows, too?
I'm actually not sure if scripts would be relying on this. There is of
course a good chance that people actually will rely on it, regardless of
what we think. If there are consistency concerns on different platforms
I'd probably have to work on that as well. Mentioning that windows isn't
affected by these changes would be a start though.
> Perhaps it's more along the lines of "this is not supposed to happen
> in practice, and when it does, humans will find it easier to debug if
> we error out hard instead of falling back to the 'if the command does
> not exist' behavior (e.g., by trying an alias next)". In other words,
> maybe this is intended as an optional nicety rather than something
> scripts would ever rely on.
Exactly. My concern was primarily the human interaction, so getting at
least some pointer to the cause of the error. Would that be nice to have
on windows as well? It probably would.
^ permalink raw reply
* Re: git version not changed after installing new version
From: Carlos Martín Nieto @ 2012-01-25 22:29 UTC (permalink / raw)
To: freefly; +Cc: git
In-Reply-To: <loom.20120125T211638-609@post.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 695 bytes --]
On Wed, 2012-01-25 at 20:20 +0000, freefly wrote:
> > Please don't cull the CC list. It's customary in the git ML to send a
> > copy of the message to each participant.
> >
>
> > There you go then. That should get set in ~/.bashrc so edit that file to
> > change the order.
> >
> > cmn
>
> I didn't the rules about ML and culling some of them
> have guidelines to reduce the noise.
> So you want me to edit the ~/.bashrc
> and put change the order of those paths ?
With the places you've installed git in, that's what you need to do. You
need to tell your shell to look at /usr/local/bin before /usr/bin,
otherwise it won't use your locally-installed git.
cmn
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* [PATCH] docs: minor grammar fixes for v1.7.9 release notes
From: Jeff King @ 2012-01-25 22:20 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Signed-off-by: Jeff King <peff@peff.net>
---
Easier to view with --color-words, of course.
I also looked through "git log --first-parent v1.7.8..master" to see if
anything had been missed. I think you hit the feature highlights pretty
well (I notice you mostly include features and not bug-fixes, which I
assume is to keep the list to a readable length).
Documentation/RelNotes/1.7.9.txt | 13 +++++++------
1 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/Documentation/RelNotes/1.7.9.txt b/Documentation/RelNotes/1.7.9.txt
index f1294b4..95320aa 100644
--- a/Documentation/RelNotes/1.7.9.txt
+++ b/Documentation/RelNotes/1.7.9.txt
@@ -12,19 +12,20 @@ Updates since v1.7.8
* Git uses gettext to translate its most common interface messages
into the user's language if translations are available and the
- locale is appropriately set. Distributors can drop in new PO files
+ locale is appropriately set. Distributors can drop new PO files
in po/ to add new translations.
- * The code to handle username/password for HTTP transaction used in
+ * The code to handle username/password for HTTP transactions used in
"git push" & "git fetch" learned to talk "credential API" to
external programs to cache or store them, to allow integration with
platform native keychain mechanisms.
- * The prompted input in the terminal use our own getpass() replacement
- when possible. HTTP transactions used to ask username without echoing
- back what was typed, but with this change you will see it as you type.
+ * The input prompts in the terminal use our own getpass() replacement
+ when possible. HTTP transactions used to ask for the username without
+ echoing back what was typed, but with this change you will see it as
+ you type.
- * The internal of "revert/cherry-pick" has been tweaked to prepare
+ * The internals of "revert/cherry-pick" have been tweaked to prepare
building more generic "sequencer" on top of the implementation that
drives them.
--
1.7.9.rc2.293.gaae2
^ permalink raw reply related
* Re: [PATCH] git-completion: workaround zsh COMPREPLY bug
From: Matthieu Moy @ 2012-01-25 22:02 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, gitster
In-Reply-To: <CAMP44s16L0GJUh4JcQgjBXUf4ftT7yQFgPy0p6zCNMnZjZGQww@mail.gmail.com>
Felipe Contreras <felipe.contreras@gmail.com> writes:
>>> + typeset -h IFS
>>
>> No time to test right now, but is this not going to
>>
>> 1) leave IFS as hidden even outside the completion script, possibly
>> affecting unrelated scripts that would need to set IFS as local and keep
>> its special effect?
>
> What special effect?
>
> Unrelated scripts can still set IFS as local.
OK, I missed the fact that typeset -h had only local effect.
>> 2) break cases where strings are to be split on \n only (e.g. see
>> "foo bar\nboz" as three possible completions "foo", "bar", "boz" instead
>> of "foo bar" and "boz"?
>
> Those cases are already broken, which is what I reported to the zsh
> mailing list. You would get "foo\ bar" and "boz", and that's after
> 4.3.12; before, compgen -W would still break the completions in 3,
> because they did 'results+=( $words )', instead of 'results+=(
> "$words" )'.
Makes sense.
I still have a minor comment: maybe part of your commit message could go
to a comment in the code as well, in particular the "Once zsh is fixed"
part, to help future contributors to actually disable the workaround
when possible in the future.
Tested-by: Matthieu Moy <Matthieu.Moy@imag.fr>
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: [PATCH] Don't search files with an unset "grep" attribute
From: Jeff King @ 2012-01-25 21:46 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Conrad Irwin, git, Nguyen Thai Ngoc Duy, Dov Grobgeld
In-Reply-To: <7vaa5d4mce.fsf@alter.siamese.dyndns.org>
On Mon, Jan 23, 2012 at 10:59:45PM -0800, Junio C Hamano wrote:
> Conrad Irwin <conrad.irwin@gmail.com> writes:
> > I used to use this approach, hooking into the "diff" attribute directly to mark
> > a file as binary, however that was clearly a hack.
>
> After thinking about this a bit more, I have to say I disagree that it is
> a hack.
I kind of agree.
The biggest problem is that the name is wrong. The "diff.*.command"
option really is about generating a diff between two blobs of a certain
type. But "diff.*.textconv" and "diff.*.binary" are really just
attributes of the file, and may or may not have to do with generating a
diff. Ditto for diff.*.funcname, I think.
You argue, and I agree, that if we are talking about attributes of the
files and not diff-specific things, then other parts of git can and
should make use of that information.
So if this was all spelled:
$ cat .gitattributes
*.pdf filetype=pdf
$ cat .git/config
[filetype "pdf"]
binary = true
textconv = pdf2txt
I think it would be a no-brainer that those type attributes should apply
to "git grep".
So maybe the first step on this path would be to introduce something
like "filetype" as a new attribute, have "diff" respect its settings,
and recommend people set up filetypes as appropriate. Or maybe that just
makes things more confusing in the long run, and we are better off
simply accepting that the name is slightly misleading. But either way,
it seems clear that git should be respecting gitattributes at the very
least to mark files as binary (and I think we already use funcname
patterns; textconv is a slightly stickier subject, so I'll address that
below).
But what I'm not sure I agree with is that the idea of "I don't want to
include path X in my grep" maps to "just mark the file as binary". For
example, in git we carry around a lot of code in compat/ that comes from
other places. I generally don't want to see grep results from
compat/nedmalloc/, because that isn't git code.
But should I mark everything in compat/nedmalloc as binary? I don't
think so. I _do_ want to see changes in nedmalloc in "git log" or "git
diff". They don't bother me because they're infrequent, and we still
want to produce regular text patches for the list when they come up. But
because nedmalloc contains a lot of lines of code (even though they
don't change a lot), it happens to produce a lot of uninteresting
matches when grepping.
> - The user has flexibility to set "diff" and "grep" independently, which
> is an unnecessary complication [*1*]; and
In the nedmalloc case, if we are to have "grep" and "diff" attributes
that behave similarly, you potentially do want to set them differently.
It would be nice to be able to treat them differently in the cases you
wanted to, but not _have_ to do so. Attribute macros can almost
implement this. You could add "-grep" to binary. But you can't (as far
as I know) do "macro=foo" to handle arbitrary diff drivers. I suspect we
could extend the rules to allow macros that take an argument and pass it
to their constituent attributes.
However, I think this is the wrong road to go down. You would want
macros like this _if_ you have grep and diff attributes that basically
do the same thing (e.g., marking a file as binary for diff versus binary
for grep). But I think that's a wrong road to go down. More likely a
file is binary or it is not binary, and the problem is conflating "file
is binary" with "I do not usually want to grep this file".
I'd much rather see grep inherit diff's file attributes unconditionally
(whether we still call them "diff" or not), and add a grep attribute
that is about "usually this is worth grepping". And then have a
tri-state command-line option for grep to either include uninteresting
things, exclude them, or to give terse output for them (mention that
there were matches in foo.c, but not each one). Probably defaulting to
terse output.
That makes the case you presented work out of the box: things marked as
binary for diffing look binary to grep, and we give the usual terse
"binary file matches" output. The user doesn't have to do anything. For
more complex cases like nedmalloc, you can still achieve the "this is
text, but it's usually boring" behavior. And if you really want to do a
thorough grep, you can just "git grep --exclude=none".
> So let's step back a bit and take a look at the handling of files for
> which you do not want to see patch output and/or you do not want to see
> grep hits, in a fictional but realisitic use scenario.
> [...]
I think this is a nice user story, and it fits in with your suggested
git behavior. But I think there are other stories, too, like the
nedmalloc one. And it would be nice to make them work without hurting
the simplicity of the case you mentioned.
> If you think about it this way from the very high level description of the
> problem, aka, end user's point of view, it is fairly clear that tying the
> "binary" attribute to "git grep" to allow us to override the built-in
> buffer_is_binary() call you see in grep.c gives the most intuitive result,
> without forcing the user to do anything more than they are already doing.
This is not a complaint about the core of your point, but rather an
aside that should be considered: how many people are really using the
binary macro attribute? Personally, I never use it, because when I mark
something with a "diff" attribute, it is because I am telling git about
a specific diff driver (usually textconv). Otherwise I don't bother
setting attributes at all, because git's binary detection tends to be
good. This leaves me without setting "-text", of course, but I don't
generally care because I don't do CRLF conversion at all.
So I think it is worth considering not just people setting "binary", but
how users of just "diff" (both "-diff" and "diff=foo") will want things
to work.
> Suppose that this binary blob firmware came with an API manual formatted
> in PDF, xyzzy.pdf, also supplied by the vendor. It is also kept in the
> repository, but again, running textual diff between updated versions of
> the PDF documentation would not be very useful. I however may have a
> textconv filter defined for it to grab the text by running pdf2ascii.
>
> Now if my "git show --textconv xyzzy.pdf" has an output line that says a
> string "XYZZY API 2.0" was added to the current version, wouldn't it be
> natural for me to expect that "git grep --textconv 'XYZZY API' xyzzy.pdf"
> to find it [*4*]?
This is an interesting concept. As a user of textconv, I already have
some specialized grep tools for matching inside binary files (e.g., I
have a tool that greps within exif tags of images). But being able to do
so with "git grep", and even at an arbitrary revision, is kind of neat.
I would worry about turning it on by default, since the results could be
misleading. In particular, your pattern "foo" might be in the binary
file but not in the textconv'd version, leading you to think you had
found all instances of "foo" but had not (or much more subtle, things
like line breaks really matter during the conversion if you are going to
be using "grep -C").
Making it available by "--textconv" seems reasonable to me, though. The
only inconsistency is that it's on by default for "git show", but would
not be for "git grep".
Perhaps I am being overly paranoid on the "misleading" bit above. It
seems to me that grep has the room to be a lot more subtle, because an
omission from the output is considered "did not match". But you could
construct equally weird scenarios for "git show" (e.g., you changed
"foo" to "bar" but that part did not appear in the textconv portion.
Which is really a quality-of-implementation issue for your textconv
filter).
> As an added bonus, if you truly want to omit _any_ hit from "git grep" for
> your minified.js files, you can easily do so. Just define a textconv
> filter that yields an empty string for them, and "grep --textconv" won't
> produce any matches, not even "Binary file ... matches".
Clever. But then you will never ever see a diff for that file, either,
because we will consider all changes to be empty (actually, I didn't
check, but you may get the diff header without any content, similar to
the stat-dirty entries).
-Peff
^ permalink raw reply
* Re: [ANNOUNCE] Git 1.7.9.rc2
From: fREW Schmidt @ 2012-01-25 21:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v39b53r8w.fsf@alter.siamese.dyndns.org>
On Tue, Jan 24, 2012 at 12:11 PM, Junio C Hamano <gitster@pobox.com> wrote:
> fREW Schmidt <frioux@gmail.com> writes:
>
>> On Wed, Jan 18, 2012 at 7:16 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>> * gitk updates accumulated since early 2011.
>>
>> Where might one go to find more detail on this one?
>
> Hmm, http://git.kernel.org/?p=git/git.git;a=shortlog, perhaps?
Ah, I actually looked there before but for some reason didn't see the
latest merge. If anyone else cares:
$ git log v1.7.9-rc2 ^v1.7.8 -- gitk-git/
commit 09bb4eb4f14c0b92baa649b2b97cda2390b84b84
Merge: 10f4eb6 811c70f
Author: Junio C Hamano <gitster@pobox.com>
Date: Fri Dec 16 22:18:42 2011 -0800
Merge git://ozlabs.org/~paulus/gitk
* git://ozlabs.org/~paulus/gitk:
gitk: Make vi-style keybindings more vi-like
gitk: Make "touching paths" search support backslashes
gitk: Show modified files with separate work tree
gitk: Simplify calculation of gitdir
gitk: Run 'git rev-parse --git-dir' only once
gitk: Put temporary directory inside .git
gitk: Fix "External diff" with separate work tree
gitk: Fix "blame parent commit" with separate work tree
gitk: Fix "show origin of this line" with separate work tree
gitk: Fix file highlight when run in subdirectory
gitk: Update copyright
gitk: When a commit contains a note, mark it with a yellow box
gitk: Remember time zones from author and commit timestamps
gitk: Remove unused $cdate array
--
fREW Schmidt
http://blog.afoolishmanifesto.com
^ permalink raw reply
* Re: git version not changed after installing new version
From: freefly @ 2012-01-25 20:23 UTC (permalink / raw)
To: git
In-Reply-To: <1327521489.31804.85.camel@centaur.lab.cmartin.tk>
Sorry about the last post, I had to prune some things from the message,
because when I was trying to post it just keep telling me that
I have more quoted text than what I have written :(
^ permalink raw reply
* Re: git version not changed after installing new version
From: freefly @ 2012-01-25 20:20 UTC (permalink / raw)
To: git
In-Reply-To: <1327521489.31804.85.camel@centaur.lab.cmartin.tk>
> Please don't cull the CC list. It's customary in the git ML to send a
> copy of the message to each participant.
>
> There you go then. That should get set in ~/.bashrc so edit that file to
> change the order.
>
> cmn
I didn't the rules about ML and culling some of them
have guidelines to reduce the noise.
So you want me to edit the ~/.bashrc
and put change the order of those paths ?
^ permalink raw reply
* Re: git version not changed after installing new version
From: Carlos Martín Nieto @ 2012-01-25 19:58 UTC (permalink / raw)
To: freefly; +Cc: git
In-Reply-To: <loom.20120125T202642-92@post.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 870 bytes --]
Please don't cull the CC list. It's customary in the git ML to send a
copy of the message to each participant.
On Wed, 2012-01-25 at 19:32 +0000, freefly wrote:
>
> > What do `which git` and `type git` say? Bash remembers where it ran a
> > command so if you install a binary to a new location, it might not find
> > it straight away.
>
> which git => /usr/bin/git
>
> type git => git is hashed (/usr/bin/git)
>
> > So the script detected that no change was needed presumably. What's your
> > $PATH and is /usr/local/bin/ before /usr/bin/?
>
> echo $PATH => /usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin:
> /usr/X11/bin:/usr/local/git/bin:/opt/local/bin
>
> from the above output it is not before /usr/bin :(
There you go then. That should get set in ~/.bashrc so edit that file to
change the order.
cmn
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* Re: git version not changed after installing new version
From: freefly @ 2012-01-25 19:32 UTC (permalink / raw)
To: git
In-Reply-To: <1327517841.31804.75.camel@centaur.lab.cmartin.tk>
> What do `which git` and `type git` say? Bash remembers where it ran a
> command so if you install a binary to a new location, it might not find
> it straight away.
which git => /usr/bin/git
type git => git is hashed (/usr/bin/git)
> So the script detected that no change was needed presumably. What's your
> $PATH and is /usr/local/bin/ before /usr/bin/?
echo $PATH => /usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin:
/usr/X11/bin:/usr/local/git/bin:/opt/local/bin
from the above output it is not before /usr/bin :(
^ permalink raw reply
* Re: [PATCH 3/5] run-command: Elaborate execvp error checking
From: Jonathan Nieder @ 2012-01-25 19:22 UTC (permalink / raw)
To: Frans Klaver; +Cc: git, Junio C. Hamano, Johannes Sixt
In-Reply-To: <op.v8motzak0aolir@keputer>
Frans Klaver wrote:
> Jonathan Nieder wrote:
>> Could you give an example?
>
> The case that triggered me to work on this. I had an incorrect entry
> in my PATH and some aliasing tests failed. The generated command
> output was something like
>
> fatal: script: Access Denied
Sorry for the lack of clarity. I meant that a (precise) "before and
after" example could make the commit message a lot easier to
understand.
[...]
>> What happens on Windows?
>
> I haven't changed anything on the windows side, so that probably
> sticks to the old behavior.
This was mostly a comment on the change description --- unless I look
at the patch, if I try this out on Windows after reading the changelog
I would end up utterly confused. For patch 5/5, it also brings up
worries about consistency --- if systems are going to be relying on a
missing #! interpreter being treated differently from a missing script
for the sake of silent_exec_failure, do the same considerations apply
on Windows, too?
Perhaps it's more along the lines of "this is not supposed to happen
in practice, and when it does, humans will find it easier to debug if
we error out hard instead of falling back to the 'if the command does
not exist' behavior (e.g., by trying an alias next)". In other words,
maybe this is intended as an optional nicety rather than something
scripts would ever rely on.
Jonathan
^ permalink raw reply
* Re: Git svn migration does not work because fatal git checkout updating paths is incompatible with switching branches
From: Carlos Martín Nieto @ 2012-01-25 19:09 UTC (permalink / raw)
To: Christine Bauers; +Cc: git
In-Reply-To: <4F20442A.1080005@gmx.de>
[-- Attachment #1: Type: text/plain, Size: 3117 bytes --]
On Wed, 2012-01-25 at 19:04 +0100, Christine Bauers wrote:
> Hi there,
>
> I´m trying to migrate a repository from svn to git which branches and
> tags with the following migration script:
>
> git svn clone --no-metadata --stdlayout --A ../users.txt
> svn://host/svn/project/subproject subproject
>
> cd subproject
> git config svn.authorsfile ../../users.txt
> git svn fetch
>
> git checkout -b branch1 remotes/branch1
> git checkout -b branch2 remotes/branch2
> git checkout -b branch3 remotes/branch3
>
> git checkout -b src_v1 remotes/tags/src
> git checkout master
> git tag src src_v1
> git branch -D src_v1
>
> git checkout -b WebContent_v1 remotes/tags/WebContent
> git checkout master
> git tag WebContent WebContent_v1
> git branch -D WebContent_v1
>
> and get the follwoing errors:
>
> W: Ignoring error from SVN, path probably does not exist: (160013):
> Filesystem has no item: Datei nicht gefunden: Revision 8966, Pfad
> »subproject«
> W: Do not be alarmed at the above message git-svn is just searching
> aggressively for old history.
> This may take a while on large repositories
> fatal: git checkout: updating paths is incompatible with switching branches.
> Did you intend to checkout 'remotes/branch1' which can not be resolved
> as commit?
> fatal: git checkout: updating paths is incompatible with switching branches.
> Did you intend to checkout 'remotes/branch2 which can not be resolved as
> commit?
> fatal: git checkout: updating paths is incompatible with switching branches.
> Did you intend to checkout 'remotes/branch3' which can not be resolved
> as commit?
> fatal: git checkout: updating paths is incompatible with switching branches.
> Did you intend to checkout 'remotes/tags/src' which can not be resolved
> as commit?
> error: pathspec 'master' did not match any file(s) known to git.
> fatal: Failed to resolve 'src_v1' as a valid ref.
> error: branch 'src_v1' not found.
> fatal: git checkout: updating paths is incompatible with switching branches.
> Did you intend to checkout 'remotes/tags/WebContent' which can not be
> resolved as commit?
> error: pathspec 'master' did not match any file(s) known to git.
> fatal: Failed to resolve 'WebContent_v1' as a valid ref.
> error: branch 'WebContent_v1' not found.
>
> How do I solve this problem?
First try to figure out where the problem happens. It could be that
git-svn isn't recognising the branches properly, or that the layout
isn't what it expects or any number of things.
What layout does the repo have? Does it correspond to what git-svn is
expecting? All those error messages come from the fact that you're
telling git some starting points that it can't find. Make sure those
exist and they have the name you're giving. What does `git branch -a`
say? You're presumably not giving us the real names, so we can't tell if
there are problems there.
If you're looking to migrate completely, something like
svn-dump-fast-export ( https://github.com/barrbrain/svn-dump-fast-export
) might get you there better.
cmn
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* Re: [PATCH 3/5] run-command: Elaborate execvp error checking
From: Johannes Sixt @ 2012-01-25 19:03 UTC (permalink / raw)
To: Frans Klaver; +Cc: git, Junio C. Hamano, Jonathan Nieder
In-Reply-To: <1327444346-6243-4-git-send-email-fransklaver@gmail.com>
Am 24.01.2012 23:32, schrieb Frans Klaver:
> +static void inspect_failure(const char *argv0, int silent_exec_failure)
> +{
> + int err = errno;
> + struct strbuf sb = STRBUF_INIT;
> +
> + /* errors not related to path */
> + if (errno == E2BIG || errno == ENOMEM)
> + die_file_error(argv0, err);
> +
> + if (strchr(argv0, '/')) {
> + if (file_exists(argv0)) {
> + strbuf_add(&sb, argv0, strlen(argv0));
> + inspect_file(&sb, err, argv0);
Can we end up here if errno == ENOENT? If so, silent_exec_failure must
be checked. (inspect_file does not return.)
> + }
> + } else {
> + char *path, *next;
> + path = getenv("PATH");
> + while (path) {
> + next = strchrnul(path, ':');
> + if (path < next)
> + strbuf_add(&sb, path, next - path);
> + else
> + strbuf_addch(&sb, '.');
> +
> + if (!*next)
> + path = NULL;
> + else
> + path = next + 1;
> +
> + if (!is_searchable(sb.buf)) {
> + strbuf_release(&sb);
> + continue;
> + }
> +
> + if (sb.len && sb.buf[sb.len - 1] != '/')
> + strbuf_addch(&sb, '/');
> + strbuf_addstr(&sb, argv0);
> +
> + if (file_exists(sb.buf))
> + inspect_file(&sb, err, argv0);
> +
> + strbuf_release(&sb);
> + }
> + }
> +
> + if (err == ENOENT) {
> + if (!silent_exec_failure)
> + error("cannot exec '%s': %s", argv0,
> + strerror(ENOENT));
> + exit(127);
> + } else {
> + die_file_error(argv0, err);
> + }
> +}
> +#endif
> +
> int start_command(struct child_process *cmd)
> {
> int need_in, need_out, need_err;
> @@ -280,14 +415,7 @@ fail_pipe:
> } else {
> execvp(cmd->argv[0], (char *const*) cmd->argv);
> }
> - if (errno == ENOENT) {
> - if (!cmd->silent_exec_failure)
> - error("cannot run %s: %s", cmd->argv[0],
> - strerror(ENOENT));
> - exit(127);
> - } else {
> - die_errno("cannot exec '%s'", cmd->argv[0]);
> - }
> + inspect_failure(cmd->argv[0], cmd->silent_exec_failure);
Isn't it important that this function calls exit(127) if we want
silent_exec_failure and errno == ENOENT? But I don't see that this
guaranteed by inspect_failure; see above.
> }
> if (cmd->pid < 0)
> error("cannot fork() for %s: %s", cmd->argv[0],
-- Hannes
^ 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