* Re: [PATCHv2 0/8] config key-parsing cleanups
From: Jonathan Nieder @ 2013-01-23 7:27 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Joachim Schmitz, René Scharfe, git
In-Reply-To: <20130123062132.GA2038@sigill.intra.peff.net>
Jeff King wrote:
> So I've re-rolled the original version
Lovely. :)
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
^ permalink raw reply
* Re: [PATCHv2 8/8] reflog: use parse_config_key in config callback
From: Junio C Hamano @ 2013-01-23 7:04 UTC (permalink / raw)
To: Jeff King; +Cc: Jonathan Nieder, Joachim Schmitz, René Scharfe, git
In-Reply-To: <20130123062737.GH5036@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> This doesn't save any lines, but does keep us from doing
> error-prone pointer arithmetic with constants.
Yeah, this and 7/8 shows that the true value of the new parse
function is not line number reduction but clarity of the calling
code. There is really no point making everybody implement "last_dot
is there, so the subsection name must be between the section name
and that last_dot" like the original before this patch.
Thanks. Will read it over again and then apply.
^ permalink raw reply
* Re: [PATCH v2 2/3] push: introduce REJECT_FETCH_FIRST and REJECT_NEEDS_FORCE
From: Jeff King @ 2013-01-23 6:56 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Chris Rorvick
In-Reply-To: <1358836230-9197-3-git-send-email-gitster@pobox.com>
On Mon, Jan 21, 2013 at 10:30:29PM -0800, Junio C Hamano wrote:
> When we push to update an existing ref, if:
>
> * we do not have the object at the tip of the remote; or
> * the object at the tip of the remote is not a commit; or
> * the object we are pushing is not a commit,
>
> there is no point suggesting to fetch, integrate and push again.
>
> If we do not have the current object at the tip of the remote, we
> should tell the user to fetch first and evaluate the situation
> before deciding what to do next.
Should we? I know that it is more correct to do so, because we do not
even know for sure that the remote object is a commit, and fetching
_might_ lead to us saying "hey, this is not something that can be
fast-forwarded".
But by far the common case will be that it _is_ a commit, and the right
thing is going to be to pull. Adding in the extra steps makes the
workflow longer and more complicated, and most of the time doesn't
matter. For example, imagine that Alice is working on "master", and when
she goes to push, she finds that Bob has already pushed his work. With
the current code, she sees:
$ git push
To ...
! [rejected] HEAD -> master (non-fast-forward)
error: failed to push some refs to '...'
hint: Updates were rejected because the tip of your current branch is behind
hint: its remote counterpart. Merge the remote changes (e.g. 'git pull')
hint: before pushing again.
and she presumably pulls, and all is well with the follow-up push.
With your patch, she sees:
$ git push
To ...
! [rejected] HEAD -> master (fetch first)
error: failed to push some refs to '...'
hint: Updates were rejected; you need to fetch the destination reference
hint: to decide what to do.
$ git fetch
...
$ git push
To ...
! [rejected] HEAD -> master (non-fast-forward)
error: failed to push some refs to '...'
hint: Updates were rejected because the tip of your current branch is behind
hint: its remote counterpart. Merge the remote changes (e.g. 'git pull')
hint: before pushing again.
hint: See the 'Note about fast-forwards' in 'git push --help' for details.
which is technically more correct (it's possible that in the second
step, she would find that Bob pushed a tree or something). But in the
common case that it is a commit, we've needlessly added two extra steps
(a fetch and another failed push), both of which involve network access
(so they are slow, and may involve Alice having to type her credentials).
Is the extra hassle in the common case worth it for the off chance that
we might give a more accurate message? Should the "fetch first" message
be some hybrid that covers both cases accurately, but still points the
user towards "git pull" (which will fail anyway if the remote ref is not
a commit)?
-Peff
^ permalink raw reply
* Re: [PATCH] all: new command used for multi-repo operations
From: Junio C Hamano @ 2013-01-23 6:52 UTC (permalink / raw)
To: Lars Hjemli; +Cc: git
In-Reply-To: <1358889019-4554-1-git-send-email-hjemli@gmail.com>
Lars Hjemli <hjemli@gmail.com> writes:
> +static struct option builtin_all_options[] = {
> + OPT_BOOLEAN('c', "clean", &only_clean, N_("only show clean repositories")),
> + OPT_BOOLEAN('d', "dirty", &only_dirty, N_("only show dirty repositories")),
> + OPT_END(),
> +};
If you were to go in the OPT_SET_INT route, that would give users
the usual "last one wins" semantics, e.g.
$ git for-each-repo --clean --dirty
will look for only dirty repositories. For completeness, we would
probably want "all" to defeat either of them, i.e.
$ git for-each-repo --clean --all
> +static int walk(struct strbuf *path, int argc, const char **argv)
> +{
> + DIR *dir;
> + struct dirent *ent;
> + size_t len;
> +
> + dir = opendir(path->buf);
> + if (!dir)
> + return errno;
> + strbuf_addstr(path, "/");
> + len = path->len;
> + while ((ent = readdir(dir))) {
> + if (!strcmp(ent->d_name, ".") || !strcmp(ent->d_name, ".."))
> + continue;
> + if (!strcmp(ent->d_name, ".git")) {
This only looks for the top of working tree. Have you considered if
this "iterate over directories and list git repositories in them"
may be useful for collection of bare repositories, and if it is, how
to go about implementing the discovery process?
> + if (ent->d_type != DT_DIR)
> + continue;
I think this is wrong.
On platforms that need a NO_D_TYPE_IN_DIRENT build, your compilation
may fail here (you would need to lstat() it yourself). See how
dir.c does this without ugly #ifdef's in the code, especially around
the use of get_dtype() and DTYPE() macro.
^ permalink raw reply
* Re: [PATCH] all: new command used for multi-repo operations
From: Junio C Hamano @ 2013-01-23 6:44 UTC (permalink / raw)
To: David Aguilar; +Cc: Lars Hjemli, git
In-Reply-To: <CAJDDKr6exXh14m08HTihxREjSFgyPT0bN1cF8eUryXJHOgFL1A@mail.gmail.com>
David Aguilar <davvid@gmail.com> writes:
>> +static int walk(struct strbuf *path, int argc, const char **argv)
>> +{
>> + DIR *dir;
>> + struct dirent *ent;
>> + size_t len;
>> +
>> + dir = opendir(path->buf);
>> + if (!dir)
>> + return errno;
>> + strbuf_addstr(path, "/");
>> + len = path->len;
>> + while ((ent = readdir(dir))) {
>> + if (!strcmp(ent->d_name, ".") || !strcmp(ent->d_name, ".."))
>> + continue;
>> + if (!strcmp(ent->d_name, ".git")) {
>> + strbuf_setlen(path, len - 1);
>> + chdir(path->buf);
>> + handle_repo(path->buf, argv);
>> + chdir(root);
>> + strbuf_addstr(path, "/");
>> + continue;
>> + }
>
> Does this section above properly handle .git files (where .git is a
> file, not a directory)?
This scans a directory $D to ask "is there '.git' in you?" and if
the answer is "yes", then hands $D (not "$D/.git") to handle_repo().
That logic will not miss a gitfile that points at the real $GIT_DIR
elsewhere.
There is a recursive call to walk() later in the same loop when the
found entry ent turns out to be a directory, and "$D/" + ent->d_name
is given to this function.
But I do not think the loop structure of this function is right. If
$D has ".git" in it, should it even try to feed other subdirectories
of $D (say "$D/a") to itself in recursion to see if $D/a/.git exists?
I think it should be more like
walk(struct strbuf *path)
{
size_t dirlen = path->len;
int has_git;
strbuf_addstr(path, "/.git");
has_git = !lstat(path->buf);
strbuf_setlen(path, dirlen);
if (has_git) {
handle_repo(path->buf);
return;
}
dir = opendir(path->buf);
while ((ent = readdir(dir))) {
... skip . and .. ...
strbuf_addstr(path, ent->d_name);
walk(path);
strbuf_setlen(path, dirlen);
}
}
The determination of has_git can be a bit fancier than a simple
!lstat() as you mentioned.
^ permalink raw reply
* Re: [PATCH v2 1/3] push: further clean up fields of "struct ref"
From: Jeff King @ 2013-01-23 6:43 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Chris Rorvick
In-Reply-To: <1358836230-9197-2-git-send-email-gitster@pobox.com>
On Mon, Jan 21, 2013 at 10:30:28PM -0800, Junio C Hamano wrote:
> The "nonfastforward" and "update" fields are only used while
> deciding what value to assign to the "status" locally in a single
> function. Remove them from the "struct ref".
>
> The "requires_force" field is not used to decide if the proposed
> update requires a --force option to succeed, or to record such a
> decision made elsewhere. It is used by status reporting code that
> the particular update was "forced". Rename it to "forced_udpate",
Typo.
> and move the code to assign to it around to further clarify how it
> is used and what it is used for.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>
> * The "update" removal in v1 has been moved to this.
>
> cache.h | 4 +---
> remote.c | 16 ++++++----------
> transport.c | 2 +-
> 3 files changed, 8 insertions(+), 14 deletions(-)
Looks much better.
I wondered briefly why nonfastforward was even there, as I recall that I
was the one who added it many years ago. It turns out that it used to
serve the purpose of the new forced_update, but Chris's series from a
few months ago split it out to "nonfastforward" and "not_forwardable",
and then added "requires_force" to give a single flag that is set in
either case.
So I think your simplification is correct; the first two can be local
variables, and the only thing that matters to carry forward is
requires_force (and I agree that forced_update is a better name).
-Peff
^ permalink raw reply
* [PATCHv2 8/8] reflog: use parse_config_key in config callback
From: Jeff King @ 2013-01-23 6:27 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jonathan Nieder, Joachim Schmitz, René Scharfe, git
In-Reply-To: <20130123062132.GA2038@sigill.intra.peff.net>
This doesn't save any lines, but does keep us from doing
error-prone pointer arithmetic with constants.
Signed-off-by: Jeff King <peff@peff.net>
---
builtin/reflog.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/builtin/reflog.c b/builtin/reflog.c
index b3c9e27..1fedf66 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -510,26 +510,27 @@ static int reflog_expire_config(const char *var, const char *value, void *cb)
static int reflog_expire_config(const char *var, const char *value, void *cb)
{
- const char *lastdot = strrchr(var, '.');
+ const char *pattern, *key;
+ int pattern_len;
unsigned long expire;
int slot;
struct reflog_expire_cfg *ent;
- if (!lastdot || prefixcmp(var, "gc."))
+ if (parse_config_key(var, "gc", &pattern, &pattern_len, &key) < 0)
return git_default_config(var, value, cb);
- if (!strcmp(lastdot, ".reflogexpire")) {
+ if (!strcmp(key, "reflogexpire")) {
slot = EXPIRE_TOTAL;
if (parse_expire_cfg_value(var, value, &expire))
return -1;
- } else if (!strcmp(lastdot, ".reflogexpireunreachable")) {
+ } else if (!strcmp(key, "reflogexpireunreachable")) {
slot = EXPIRE_UNREACH;
if (parse_expire_cfg_value(var, value, &expire))
return -1;
} else
return git_default_config(var, value, cb);
- if (lastdot == var + 2) {
+ if (!pattern) {
switch (slot) {
case EXPIRE_TOTAL:
default_reflog_expire = expire;
@@ -541,7 +542,7 @@ static int reflog_expire_config(const char *var, const char *value, void *cb)
return 0;
}
- ent = find_cfg_ent(var + 3, lastdot - (var+3));
+ ent = find_cfg_ent(pattern, pattern_len);
if (!ent)
return -1;
switch (slot) {
--
1.8.0.2.15.g815dc66
^ permalink raw reply related
* [PATCHv2 7/8] help: use parse_config_key for man config
From: Jeff King @ 2013-01-23 6:27 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jonathan Nieder, Joachim Schmitz, René Scharfe, git
In-Reply-To: <20130123062132.GA2038@sigill.intra.peff.net>
The resulting code ends up about the same length, but it is
a little more self-explanatory. It now explicitly documents
and checks the pre-condition that the incoming var starts
with "man.", and drops the magic offset "4".
Signed-off-by: Jeff King <peff@peff.net>
---
builtin/help.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/builtin/help.c b/builtin/help.c
index bd86253..04cb77d 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -237,21 +237,21 @@ static int add_man_viewer_info(const char *var, const char *value)
static int add_man_viewer_info(const char *var, const char *value)
{
- const char *name = var + 4;
- const char *subkey = strrchr(name, '.');
+ const char *name, *subkey;
+ int namelen;
- if (!subkey)
+ if (parse_config_key(var, "man", &name, &namelen, &subkey) < 0 || !name)
return 0;
- if (!strcmp(subkey, ".path")) {
+ if (!strcmp(subkey, "path")) {
if (!value)
return config_error_nonbool(var);
- return add_man_viewer_path(name, subkey - name, value);
+ return add_man_viewer_path(name, namelen, value);
}
- if (!strcmp(subkey, ".cmd")) {
+ if (!strcmp(subkey, "cmd")) {
if (!value)
return config_error_nonbool(var);
- return add_man_viewer_cmd(name, subkey - name, value);
+ return add_man_viewer_cmd(name, namelen, value);
}
return 0;
--
1.8.0.2.15.g815dc66
^ permalink raw reply related
* [PATCHv2 6/8] submodule: simplify memory handling in config parsing
From: Jeff King @ 2013-01-23 6:26 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jonathan Nieder, Joachim Schmitz, René Scharfe, git
In-Reply-To: <20130123062132.GA2038@sigill.intra.peff.net>
We keep a strbuf for the name of the submodule, even though
we only ever add one string to it. Let's just use xmemdupz
instead, which is slightly more efficient and makes it
easier to follow what is going on.
Unfortunately, we still end up having to deal with some
memory ownership issues in some code branches, as we have to
allocate the string in order to do a string list lookup, and
then only sometimes want to hand ownership of that string
over to the string_list. Still, making that explicit in the
code (as opposed to sometimes detaching the strbuf, and then
always releasing it) makes it a little more obvious what is
going on.
Signed-off-by: Jeff King <peff@peff.net>
---
submodule.c | 30 ++++++++++++++----------------
1 file changed, 14 insertions(+), 16 deletions(-)
diff --git a/submodule.c b/submodule.c
index 25413de..9ba1496 100644
--- a/submodule.c
+++ b/submodule.c
@@ -127,7 +127,6 @@ int parse_submodule_config_option(const char *var, const char *value)
int parse_submodule_config_option(const char *var, const char *value)
{
struct string_list_item *config;
- struct strbuf submodname = STRBUF_INIT;
const char *name, *key;
int namelen;
@@ -135,37 +134,36 @@ int parse_submodule_config_option(const char *var, const char *value)
return 0;
if (!strcmp(key, "path")) {
- strbuf_add(&submodname, name, namelen);
config = unsorted_string_list_lookup(&config_name_for_path, value);
if (config)
free(config->util);
else
config = string_list_append(&config_name_for_path, xstrdup(value));
- config->util = strbuf_detach(&submodname, NULL);
- strbuf_release(&submodname);
+ config->util = xmemdupz(name, namelen);
} else if (!strcmp(key, "fetchrecursesubmodules")) {
- strbuf_add(&submodname, name, namelen);
- config = unsorted_string_list_lookup(&config_fetch_recurse_submodules_for_name, submodname.buf);
+ char *name_cstr = xmemdupz(name, namelen);
+ config = unsorted_string_list_lookup(&config_fetch_recurse_submodules_for_name, name_cstr);
if (!config)
- config = string_list_append(&config_fetch_recurse_submodules_for_name,
- strbuf_detach(&submodname, NULL));
+ config = string_list_append(&config_fetch_recurse_submodules_for_name, name_cstr);
+ else
+ free(name_cstr);
config->util = (void *)(intptr_t)parse_fetch_recurse_submodules_arg(var, value);
- strbuf_release(&submodname);
} else if (!strcmp(key, "ignore")) {
+ char *name_cstr;
+
if (strcmp(value, "untracked") && strcmp(value, "dirty") &&
strcmp(value, "all") && strcmp(value, "none")) {
warning("Invalid parameter \"%s\" for config option \"submodule.%s.ignore\"", value, var);
return 0;
}
- strbuf_add(&submodname, name, namelen);
- config = unsorted_string_list_lookup(&config_ignore_for_name, submodname.buf);
- if (config)
+ name_cstr = xmemdupz(name, namelen);
+ config = unsorted_string_list_lookup(&config_ignore_for_name, name_cstr);
+ if (config) {
free(config->util);
- else
- config = string_list_append(&config_ignore_for_name,
- strbuf_detach(&submodname, NULL));
- strbuf_release(&submodname);
+ free(name_cstr);
+ } else
+ config = string_list_append(&config_ignore_for_name, name_cstr);
config->util = xstrdup(value);
return 0;
}
--
1.8.0.2.15.g815dc66
^ permalink raw reply related
* [PATCHv2 5/8] submodule: use parse_config_key when parsing config
From: Jeff King @ 2013-01-23 6:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jonathan Nieder, Joachim Schmitz, René Scharfe, git
In-Reply-To: <20130123062132.GA2038@sigill.intra.peff.net>
This makes the code a lot simpler to read by dropping a
whole bunch of constant offsets.
As a bonus, it means we also feed the whole config variable
name to our error functions:
[before]
$ git -c submodule.foo.fetchrecursesubmodules=bogus checkout
fatal: bad foo.fetchrecursesubmodules argument: bogus
[after]
$ git -c submodule.foo.fetchrecursesubmodules=bogus checkout
fatal: bad submodule.foo.fetchrecursesubmodules argument: bogus
Signed-off-by: Jeff King <peff@peff.net>
---
submodule.c | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/submodule.c b/submodule.c
index 2f55436..25413de 100644
--- a/submodule.c
+++ b/submodule.c
@@ -126,15 +126,16 @@ int parse_submodule_config_option(const char *var, const char *value)
int parse_submodule_config_option(const char *var, const char *value)
{
- int len;
struct string_list_item *config;
struct strbuf submodname = STRBUF_INIT;
+ const char *name, *key;
+ int namelen;
- var += 10; /* Skip "submodule." */
+ if (parse_config_key(var, "submodule", &name, &namelen, &key) < 0 || !name)
+ return 0;
- len = strlen(var);
- if ((len > 5) && !strcmp(var + len - 5, ".path")) {
- strbuf_add(&submodname, var, len - 5);
+ if (!strcmp(key, "path")) {
+ strbuf_add(&submodname, name, namelen);
config = unsorted_string_list_lookup(&config_name_for_path, value);
if (config)
free(config->util);
@@ -142,22 +143,22 @@ int parse_submodule_config_option(const char *var, const char *value)
config = string_list_append(&config_name_for_path, xstrdup(value));
config->util = strbuf_detach(&submodname, NULL);
strbuf_release(&submodname);
- } else if ((len > 23) && !strcmp(var + len - 23, ".fetchrecursesubmodules")) {
- strbuf_add(&submodname, var, len - 23);
+ } else if (!strcmp(key, "fetchrecursesubmodules")) {
+ strbuf_add(&submodname, name, namelen);
config = unsorted_string_list_lookup(&config_fetch_recurse_submodules_for_name, submodname.buf);
if (!config)
config = string_list_append(&config_fetch_recurse_submodules_for_name,
strbuf_detach(&submodname, NULL));
config->util = (void *)(intptr_t)parse_fetch_recurse_submodules_arg(var, value);
strbuf_release(&submodname);
- } else if ((len > 7) && !strcmp(var + len - 7, ".ignore")) {
+ } else if (!strcmp(key, "ignore")) {
if (strcmp(value, "untracked") && strcmp(value, "dirty") &&
strcmp(value, "all") && strcmp(value, "none")) {
warning("Invalid parameter \"%s\" for config option \"submodule.%s.ignore\"", value, var);
return 0;
}
- strbuf_add(&submodname, var, len - 7);
+ strbuf_add(&submodname, name, namelen);
config = unsorted_string_list_lookup(&config_ignore_for_name, submodname.buf);
if (config)
free(config->util);
--
1.8.0.2.15.g815dc66
^ permalink raw reply related
* [PATCHv2 4/8] userdiff: drop parse_driver function
From: Jeff King @ 2013-01-23 6:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jonathan Nieder, Joachim Schmitz, René Scharfe, git
In-Reply-To: <20130123062132.GA2038@sigill.intra.peff.net>
When we parse userdiff config, we generally assume that
diff.name.key
will affect the "key" value of the "name" driver. However,
without confirming that the key is a valid userdiff key, we
may accidentally conflict with the ancient "diff.color.*"
namespace. The current code is careful not to even create a
driver struct if we do not see a key that is known by the
diff-driver code.
However, this carefulness is unnecessary; the default driver
with no keys set behaves exactly the same as having no
driver at all. We can simply set up the driver struct as
soon as we see we have a config key that looks like a
driver. This makes the code a bit more readable.
Signed-off-by: Jeff King <peff@peff.net>
---
userdiff.c | 50 +++++++++++++++++++++-----------------------------
1 file changed, 21 insertions(+), 29 deletions(-)
diff --git a/userdiff.c b/userdiff.c
index a4ea1e9..ea43a03 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -184,28 +184,6 @@ static struct userdiff_driver *userdiff_find_by_namelen(const char *k, int len)
return NULL;
}
-static struct userdiff_driver *parse_driver(const char *var,
- const char *value, const char *type)
-{
- struct userdiff_driver *drv;
- const char *name, *key;
- int namelen;
-
- if (parse_config_key(var, "diff", &name, &namelen, &key) < 0 ||
- !name || strcmp(type, key))
- return NULL;
-
- drv = userdiff_find_by_namelen(name, namelen);
- if (!drv) {
- ALLOC_GROW(drivers, ndrivers+1, drivers_alloc);
- drv = &drivers[ndrivers++];
- memset(drv, 0, sizeof(*drv));
- drv->name = xmemdupz(name, namelen);
- drv->binary = -1;
- }
- return drv;
-}
-
static int parse_funcname(struct userdiff_funcname *f, const char *k,
const char *v, int cflags)
{
@@ -233,20 +211,34 @@ int userdiff_config(const char *k, const char *v)
int userdiff_config(const char *k, const char *v)
{
struct userdiff_driver *drv;
+ const char *name, *type;
+ int namelen;
+
+ if (parse_config_key(k, "diff", &name, &namelen, &type) || !name)
+ return 0;
+
+ drv = userdiff_find_by_namelen(name, namelen);
+ if (!drv) {
+ ALLOC_GROW(drivers, ndrivers+1, drivers_alloc);
+ drv = &drivers[ndrivers++];
+ memset(drv, 0, sizeof(*drv));
+ drv->name = xmemdupz(name, namelen);
+ drv->binary = -1;
+ }
- if ((drv = parse_driver(k, v, "funcname")))
+ if (!strcmp(type, "funcname"))
return parse_funcname(&drv->funcname, k, v, 0);
- if ((drv = parse_driver(k, v, "xfuncname")))
+ if (!strcmp(type, "xfuncname"))
return parse_funcname(&drv->funcname, k, v, REG_EXTENDED);
- if ((drv = parse_driver(k, v, "binary")))
+ if (!strcmp(type, "binary"))
return parse_tristate(&drv->binary, k, v);
- if ((drv = parse_driver(k, v, "command")))
+ if (!strcmp(type, "command"))
return git_config_string(&drv->external, k, v);
- if ((drv = parse_driver(k, v, "textconv")))
+ if (!strcmp(type, "textconv"))
return git_config_string(&drv->textconv, k, v);
- if ((drv = parse_driver(k, v, "cachetextconv")))
+ if (!strcmp(type, "cachetextconv"))
return parse_bool(&drv->textconv_want_cache, k, v);
- if ((drv = parse_driver(k, v, "wordregex")))
+ if (!strcmp(type, "wordregex"))
return git_config_string(&drv->word_regex, k, v);
return 0;
--
1.8.0.2.15.g815dc66
^ permalink raw reply related
* [PATCHv2 3/8] convert some config callbacks to parse_config_key
From: Jeff King @ 2013-01-23 6:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jonathan Nieder, Joachim Schmitz, René Scharfe, git
In-Reply-To: <20130123062132.GA2038@sigill.intra.peff.net>
These callers can drop some inline pointer arithmetic and
magic offset constants, making them more readable and less
error-prone (those constants had to match the lengths of
strings, but there is no automatic verification of that
fact).
The "ep" pointer (presumably for "end pointer"), which
points to the final key segment of the config variable, is
given the more standard name "key" to describe its function
rather than its derivation.
Signed-off-by: Jeff King <peff@peff.net>
---
convert.c | 14 +++++---------
ll-merge.c | 14 +++++---------
userdiff.c | 13 +++----------
3 files changed, 13 insertions(+), 28 deletions(-)
diff --git a/convert.c b/convert.c
index 6602155..3520252 100644
--- a/convert.c
+++ b/convert.c
@@ -457,7 +457,7 @@ static int read_convert_config(const char *var, const char *value, void *cb)
static int read_convert_config(const char *var, const char *value, void *cb)
{
- const char *ep, *name;
+ const char *key, *name;
int namelen;
struct convert_driver *drv;
@@ -465,10 +465,8 @@ static int read_convert_config(const char *var, const char *value, void *cb)
* External conversion drivers are configured using
* "filter.<name>.variable".
*/
- if (prefixcmp(var, "filter.") || (ep = strrchr(var, '.')) == var + 6)
+ if (parse_config_key(var, "filter", &name, &namelen, &key) < 0 || !name)
return 0;
- name = var + 7;
- namelen = ep - name;
for (drv = user_convert; drv; drv = drv->next)
if (!strncmp(drv->name, name, namelen) && !drv->name[namelen])
break;
@@ -479,8 +477,6 @@ static int read_convert_config(const char *var, const char *value, void *cb)
user_convert_tail = &(drv->next);
}
- ep++;
-
/*
* filter.<name>.smudge and filter.<name>.clean specifies
* the command line:
@@ -490,13 +486,13 @@ static int read_convert_config(const char *var, const char *value, void *cb)
* The command-line will not be interpolated in any way.
*/
- if (!strcmp("smudge", ep))
+ if (!strcmp("smudge", key))
return git_config_string(&drv->smudge, var, value);
- if (!strcmp("clean", ep))
+ if (!strcmp("clean", key))
return git_config_string(&drv->clean, var, value);
- if (!strcmp("required", ep)) {
+ if (!strcmp("required", key)) {
drv->required = git_config_bool(var, value);
return 0;
}
diff --git a/ll-merge.c b/ll-merge.c
index acea33b..fb61ea6 100644
--- a/ll-merge.c
+++ b/ll-merge.c
@@ -222,7 +222,7 @@ static int read_merge_config(const char *var, const char *value, void *cb)
static int read_merge_config(const char *var, const char *value, void *cb)
{
struct ll_merge_driver *fn;
- const char *ep, *name;
+ const char *key, *name;
int namelen;
if (!strcmp(var, "merge.default")) {
@@ -236,15 +236,13 @@ static int read_merge_config(const char *var, const char *value, void *cb)
* especially, we do not want to look at variables such as
* "merge.summary", "merge.tool", and "merge.verbosity".
*/
- if (prefixcmp(var, "merge.") || (ep = strrchr(var, '.')) == var + 5)
+ if (parse_config_key(var, "merge", &name, &namelen, &key) < 0 || !name)
return 0;
/*
* Find existing one as we might be processing merge.<name>.var2
* after seeing merge.<name>.var1.
*/
- name = var + 6;
- namelen = ep - name;
for (fn = ll_user_merge; fn; fn = fn->next)
if (!strncmp(fn->name, name, namelen) && !fn->name[namelen])
break;
@@ -256,16 +254,14 @@ static int read_merge_config(const char *var, const char *value, void *cb)
ll_user_merge_tail = &(fn->next);
}
- ep++;
-
- if (!strcmp("name", ep)) {
+ if (!strcmp("name", key)) {
if (!value)
return error("%s: lacks value", var);
fn->description = xstrdup(value);
return 0;
}
- if (!strcmp("driver", ep)) {
+ if (!strcmp("driver", key)) {
if (!value)
return error("%s: lacks value", var);
/*
@@ -289,7 +285,7 @@ static int read_merge_config(const char *var, const char *value, void *cb)
return 0;
}
- if (!strcmp("recursive", ep)) {
+ if (!strcmp("recursive", key)) {
if (!value)
return error("%s: lacks value", var);
fn->recursive = xstrdup(value);
diff --git a/userdiff.c b/userdiff.c
index ed958ef..a4ea1e9 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -188,20 +188,13 @@ static struct userdiff_driver *parse_driver(const char *var,
const char *value, const char *type)
{
struct userdiff_driver *drv;
- const char *dot;
- const char *name;
+ const char *name, *key;
int namelen;
- if (prefixcmp(var, "diff."))
- return NULL;
- dot = strrchr(var, '.');
- if (dot == var + 4)
- return NULL;
- if (strcmp(type, dot+1))
+ if (parse_config_key(var, "diff", &name, &namelen, &key) < 0 ||
+ !name || strcmp(type, key))
return NULL;
- name = var + 5;
- namelen = dot - name;
drv = userdiff_find_by_namelen(name, namelen);
if (!drv) {
ALLOC_GROW(drivers, ndrivers+1, drivers_alloc);
--
1.8.0.2.15.g815dc66
^ permalink raw reply related
* [PATCHv2 2/8] archive-tar: use parse_config_key when parsing config
From: Jeff King @ 2013-01-23 6:23 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jonathan Nieder, Joachim Schmitz, René Scharfe, git
In-Reply-To: <20130123062132.GA2038@sigill.intra.peff.net>
This is fewer lines of code, but more importantly, fixes a
bogus pointer offset. We are looking for "tar." in the
section, but later assume that the dot we found is at offset
9, not 3. This is a holdover from an earlier iteration of
767cf45 which called the section "tarfilter".
As a result, we could erroneously reject some filters with
dots in their name, as well as read uninitialized memory.
Reported by (and test by) René Scharfe.
Signed-off-by: Jeff King <peff@peff.net>
---
archive-tar.c | 10 +---------
t/t5000-tar-tree.sh | 3 ++-
2 files changed, 3 insertions(+), 10 deletions(-)
diff --git a/archive-tar.c b/archive-tar.c
index d1cce46..719b629 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -327,20 +327,12 @@ static int tar_filter_config(const char *var, const char *value, void *data)
static int tar_filter_config(const char *var, const char *value, void *data)
{
struct archiver *ar;
- const char *dot;
const char *name;
const char *type;
int namelen;
- if (prefixcmp(var, "tar."))
+ if (parse_config_key(var, "tar", &name, &namelen, &type) < 0 || !name)
return 0;
- dot = strrchr(var, '.');
- if (dot == var + 9)
- return 0;
-
- name = var + 4;
- namelen = dot - name;
- type = dot + 1;
ar = find_tar_filter(name, namelen);
if (!ar) {
diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh
index e7c240f..3fbd366 100755
--- a/t/t5000-tar-tree.sh
+++ b/t/t5000-tar-tree.sh
@@ -212,7 +212,8 @@ test_expect_success 'setup tar filters' '
test_expect_success 'setup tar filters' '
git config tar.tar.foo.command "tr ab ba" &&
git config tar.bar.command "tr ab ba" &&
- git config tar.bar.remote true
+ git config tar.bar.remote true &&
+ git config tar.invalid baz
'
test_expect_success 'archive --list mentions user filter' '
--
1.8.0.2.15.g815dc66
^ permalink raw reply related
* [PATCHv2 1/8] config: add helper function for parsing key names
From: Jeff King @ 2013-01-23 6:23 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jonathan Nieder, Joachim Schmitz, René Scharfe, git
In-Reply-To: <20130123062132.GA2038@sigill.intra.peff.net>
The config callback functions get keys of the general form:
section.subsection.key
(where the subsection may be contain arbitrary data, or may
be missing). For matching keys without subsections, it is
simple enough to call "strcmp". Matching keys with
subsections is a little more complicated, and each callback
does it in an ad-hoc way, usually involving error-prone
pointer arithmetic.
Let's provide a helper that keeps the pointer arithmetic all
in one place.
Signed-off-by: Jeff King <peff@peff.net>
---
cache.h | 15 +++++++++++++++
config.c | 33 +++++++++++++++++++++++++++++++++
2 files changed, 48 insertions(+)
diff --git a/cache.h b/cache.h
index c257953..b19305b 100644
--- a/cache.h
+++ b/cache.h
@@ -1164,6 +1164,21 @@ extern int git_config_include(const char *name, const char *value, void *data);
#define CONFIG_INCLUDE_INIT { 0 }
extern int git_config_include(const char *name, const char *value, void *data);
+/*
+ * Match and parse a config key of the form:
+ *
+ * section.(subsection.)?key
+ *
+ * (i.e., what gets handed to a config_fn_t). The caller provides the section;
+ * we return -1 if it does not match, 0 otherwise. The subsection and key
+ * out-parameters are filled by the function (and subsection is NULL if it is
+ * missing).
+ */
+extern int parse_config_key(const char *var,
+ const char *section,
+ const char **subsection, int *subsection_len,
+ const char **key);
+
extern int committer_ident_sufficiently_given(void);
extern int author_ident_sufficiently_given(void);
diff --git a/config.c b/config.c
index 7b444b6..11bd4d8 100644
--- a/config.c
+++ b/config.c
@@ -1667,3 +1667,36 @@ int config_error_nonbool(const char *var)
{
return error("Missing value for '%s'", var);
}
+
+int parse_config_key(const char *var,
+ const char *section,
+ const char **subsection, int *subsection_len,
+ const char **key)
+{
+ int section_len = strlen(section);
+ const char *dot;
+
+ /* Does it start with "section." ? */
+ if (prefixcmp(var, section) || var[section_len] != '.')
+ return -1;
+
+ /*
+ * Find the key; we don't know yet if we have a subsection, but we must
+ * parse backwards from the end, since the subsection may have dots in
+ * it, too.
+ */
+ dot = strrchr(var, '.');
+ *key = dot + 1;
+
+ /* Did we have a subsection at all? */
+ if (dot == var + section_len) {
+ *subsection = NULL;
+ *subsection_len = 0;
+ }
+ else {
+ *subsection = var + section_len + 1;
+ *subsection_len = dot - *subsection;
+ }
+
+ return 0;
+}
--
1.8.0.2.15.g815dc66
^ permalink raw reply related
* [PATCHv2 0/8] config key-parsing cleanups
From: Jeff King @ 2013-01-23 6:21 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jonathan Nieder, Joachim Schmitz, René Scharfe, git
In-Reply-To: <7va9s6qkkz.fsf@alter.siamese.dyndns.org>
On Fri, Jan 18, 2013 at 12:53:32PM -0800, Junio C Hamano wrote:
> >> ... did you have any comment on
> >> the "struct config_key" alternative I sent as a follow-up?
> >
> > I did read it but I cannot say I did so very carefully. My gut
> > reaction was that the "take the variable name and section name,
> > return the subsection name pointer and length, if there is any, and
> > the key" made it readable enough. The proposed interface to make
> > and lend a copy to the caller does make it more readble, but I do
> > not know if that is worth doing. Neutral-to-slightly-in-favor, I
> > would say.
>
> Now I re-read that "struct config_key" thing, I would have to say
> that the idea of giving split and NUL-terminated strings to the
> callers is good, but the "cheat" looks somewhat brittle for all the
> reasons that come from using a static buffer (which you already
> mentioned). As I do not offhand think of a better alternative, I'd
> say we leave it for another day.
OK. I had the feeling if the config parser provided it to the caller
that more sites could take advantage of it (without adding too many
lines to call the parsing function). But looking again, there aren't
that many sites that would benefit. E.g., git_daemon_config in daemon.c
could use it to avoid using a constant offset. But the current code
there is not hard to read, and saving a few characters there is not
worth the complexity.
So I've re-rolled the original version, taking into account the comments
from you and Jonathan. I also clarified a few of the commit messages,
and modified two more sites to use the new function.
[1/8]: config: add helper function for parsing key names
Same as before, but now called parse_config_key.
[2/8]: archive-tar: use parse_config_key when parsing config
Same (rebased for new name, of course).
[3/8]: convert some config callbacks to parse_config_key
Tweaked confusing "ep" variable name. Fixed missing "!name" check in
userdiff code (which gets removed in the next patch anyway).
[4/8]: userdiff: drop parse_driver function
[5/8]: submodule: use parse_config_key when parsing config
[6/8]: submodule: simplify memory handling in config parsing
Same.
[7/8]: help: use parse_config_key for man config
[8/8]: reflog: use parse_config_key in config callback
Two new callsites. I split these out because unlike the ones in 3/8,
they do not benefit from a reduction in lines of code. However, I think
the results are still more readable. You can judge for yourself; drop
them if you disagree. Or feel free to squash them into 3/8 if that makes
more sense.
-Peff
^ permalink raw reply
* Re: [PATCH v3 3/6] Change 'git' to 'Git' whenever the whole system is referred to #2
From: Junio C Hamano @ 2013-01-23 4:17 UTC (permalink / raw)
To: Thomas Ackermann; +Cc: git, David Aguilar
In-Reply-To: <CAJDDKr4fnUp_35ni72XJS_NSp4jxbvQPENLnk3AhFv2FBg3DTg@mail.gmail.com>
David Aguilar <davvid@gmail.com> writes:
> On Mon, Jan 21, 2013 at 11:19 AM, Thomas Ackermann <th.acker@arcor.de> wrote:
>> diff --git a/Documentation/config.txt b/Documentation/config.txt
>> index b87f744..5a831ad2 100644
>> --- a/Documentation/config.txt
>> +++ b/Documentation/config.txt
>> push.default::
>> - Defines the action git push should take if no refspec is given
>> + Defines the action Git push should take if no refspec is given
>> on the command line, no refspec is configured in the remote, and
>> no refspec is implied by any of the options given on the command
>> line. Possible values are:
>
> This should probably be "git push" in double quotes.
Yeah, or no quotes, or `git push` even.
I've pushed the result of my nitpicks I sent so far as part of the
'pu' branch, on topic 'ta/doc-no-small-caps'. Its tip is now at
bfb8e1e (fixup! Change 'git' to 'Git' whenever the whole system is
referred to #4, 2013-01-22).
Thomas, I do not want to see many rounds of entire rerolls of this
series on the list (nobody will look at the whole series multiple
times with fine toothed comb). I do not think you want to do that
either. Can you collect remaining fixups like David's message, turn
them into patch form when you have collected enough to be reviewed
in one sitting (say, a patchfile at around 200 lines), and send them
over to the list to apply on top of the tree of that commit?
After a week or so, I'll squash the series into two commits (the
first one turns 'GIT' into 'Git', the second one turns selected
'git' into 'Git'), and merge the result to 'next'.
^ permalink raw reply
* Re: [PATCH v3 3/6] Change 'git' to 'Git' whenever the whole system is referred to #2
From: David Aguilar @ 2013-01-23 3:44 UTC (permalink / raw)
To: Thomas Ackermann; +Cc: gitster, git
In-Reply-To: <2009548606.632825.1358795980319.JavaMail.ngmail@webmail20.arcor-online.net>
On Mon, Jan 21, 2013 at 11:19 AM, Thomas Ackermann <th.acker@arcor.de> wrote:
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index b87f744..5a831ad2 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> push.default::
> - Defines the action git push should take if no refspec is given
> + Defines the action Git push should take if no refspec is given
> on the command line, no refspec is configured in the remote, and
> no refspec is implied by any of the options given on the command
> line. Possible values are:
This should probably be "git push" in double quotes.
--
David
^ permalink raw reply
* Re: [PATCH v3 5/6] Change 'git' to 'Git' whenever the whole system is referred to #4
From: Junio C Hamano @ 2013-01-23 3:16 UTC (permalink / raw)
To: Thomas Ackermann; +Cc: davvid, git
In-Reply-To: <1199035912.632874.1358796172804.JavaMail.ngmail@webmail20.arcor-online.net>
Leftover in paths touched by [5/6].
Documentation/gitcvs-migration.txt | 2 +-
Documentation/technical/api-builtin.txt | 2 +-
Documentation/technical/api-credentials.txt | 4 ++--
Documentation/technical/api-remote.txt | 2 +-
Documentation/urls.txt | 2 +-
Documentation/user-manual.txt | 4 ++--
6 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/Documentation/gitcvs-migration.txt b/Documentation/gitcvs-migration.txt
index 2934ac2..5ab5b07 100644
--- a/Documentation/gitcvs-migration.txt
+++ b/Documentation/gitcvs-migration.txt
@@ -3,7 +3,7 @@ gitcvs-migration(7)
NAME
----
-gitcvs-migration - git for CVS users
+gitcvs-migration - Git for CVS users
SYNOPSIS
--------
diff --git a/Documentation/technical/api-builtin.txt b/Documentation/technical/api-builtin.txt
index 2d27ff1..4a4228b 100644
--- a/Documentation/technical/api-builtin.txt
+++ b/Documentation/technical/api-builtin.txt
@@ -5,7 +5,7 @@ Adding a new built-in
---------------------
There are 4 things to do to add a built-in command implementation to
-git:
+Git:
. Define the implementation of the built-in command `foo` with
signature:
diff --git a/Documentation/technical/api-credentials.txt b/Documentation/technical/api-credentials.txt
index f0c39e1..516fda7 100644
--- a/Documentation/technical/api-credentials.txt
+++ b/Documentation/technical/api-credentials.txt
@@ -18,7 +18,7 @@ Typical setup
------------
+-----------------------+
-| git code (C) |--- to server requiring --->
+| Git code (C) |--- to server requiring --->
| | authentication
|.......................|
| C credential API |--- prompt ---> User
@@ -27,7 +27,7 @@ Typical setup
| pipe |
| v
+-----------------------+
-| git credential helper |
+| Git credential helper |
+-----------------------+
------------
diff --git a/Documentation/technical/api-remote.txt b/Documentation/technical/api-remote.txt
index 2819d3a..4be8776 100644
--- a/Documentation/technical/api-remote.txt
+++ b/Documentation/technical/api-remote.txt
@@ -45,7 +45,7 @@ struct remote
`receivepack`, `uploadpack`::
The configured helper programs to run on the remote side, for
- git-native protocols.
+ Git-native protocols.
`http_proxy`::
diff --git a/Documentation/urls.txt b/Documentation/urls.txt
index a2cf68b..539c0a0 100644
--- a/Documentation/urls.txt
+++ b/Documentation/urls.txt
@@ -29,7 +29,7 @@ The ssh and git protocols additionally support ~username expansion:
- git://host.xz{startsb}:port{endsb}/~{startsb}user{endsb}/path/to/repo.git/
- {startsb}user@{endsb}host.xz:/~{startsb}user{endsb}/path/to/repo.git/
-For local repositories, also supported by git natively, the following
+For local repositories, also supported by Git natively, the following
syntaxes may be used:
- /path/to/repo.git/
diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
index dda262a..5077e7c 100644
--- a/Documentation/user-manual.txt
+++ b/Documentation/user-manual.txt
@@ -1439,7 +1439,7 @@ fundamentally different ways to fix the problem:
2. You can go back and modify the old commit. You should
never do this if you have already made the history public;
- git does not normally expect the "history" of a project to
+ Git does not normally expect the "history" of a project to
change, and cannot correctly perform repeated merges from
a branch that has had its history changed.
@@ -3671,7 +3671,7 @@ Unable to checkout '261dfac35cb99d380eb966e102c1197139f7fa24' in submodule path
In older Git versions it could be easily forgotten to commit new or modified
files in a submodule, which silently leads to similar problems as not pushing
-the submodule changes. Starting with git 1.7.0 both "git status" and "git diff"
+the submodule changes. Starting with Git 1.7.0 both "git status" and "git diff"
in the superproject show submodules as modified when they contain new or
modified files to protect against accidentally committing such a state. "git
diff" will also add a "-dirty" to the work tree side when generating patch
--
1.8.1.1.507.g1754052
^ permalink raw reply related
* Re: [PATCH v3 0/2] Make git-svn work with gitdir links
From: Junio C Hamano @ 2013-01-23 2:53 UTC (permalink / raw)
To: Eric Wong; +Cc: Barry Wardell, git
In-Reply-To: <20130123023235.GA24135@dcvr.yhbt.net>
Eric Wong <normalperson@yhbt.net> writes:
> `git rev-parse --show-cdup` outputs nothing if GIT_DIR is set,
> so I unset GIT_DIR temporarily.
>
> I'm not sure why --show-cdup behaves like this, though..
Setting GIT_DIR is to say "That is the directory that has the
repository objects and refs; I am letting you know the location
explicitly because it does not have any relation with the location
of the working tree. The $(cwd) is at the root of the working
tree".
If you want to say "That is the directory that has metainformation,
and that other one is the root of the working tree", you use
GIT_WORK_TREE to name the latter.
So by definition, if you only set GIT_DIR without setting
GIT_WORK_TREE, show-cdup must say "you are already at the top".
>
> Does squashing this on top of your changes fix all your failures?
> I plan on squashing both your changes together with the below:
>
> diff --git a/git-svn.perl b/git-svn.perl
> index c232798..e5bd292 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -332,11 +332,13 @@ if ($cmd && $cmd =~ /(?:clone|init|multi-init)$/) {
> $ENV{GIT_DIR} = command_oneline([qw/rev-parse --git-dir/]);
> } "Unable to find .git directory\n";
> my $cdup = undef;
> + my $git_dir = delete $ENV{GIT_DIR};
> git_cmd_try {
> $cdup = command_oneline(qw/rev-parse --show-cdup/);
> chomp $cdup if ($cdup);
> $cdup = "." unless ($cdup && length $cdup);
> - } "Already at toplevel, but $ENV{GIT_DIR} not found\n";
> + } "Already at toplevel, but $git_dir not found\n";
> + $ENV{GIT_DIR} = $git_dir;
> chdir $cdup or die "Unable to chdir up to '$cdup'\n";
> $_repository = Git->repository(Repository => $ENV{GIT_DIR});
> }
^ permalink raw reply
* Bug in EOL conversion?
From: Stefan Norgren @ 2013-01-23 2:44 UTC (permalink / raw)
To: git
Hi,
The EOL conversion does not behave as indicated by output message from
add and commit. Here is my test case executed on Windows 7 64 bit.
$ git --version
git version 1.8.0.msysgit.0
$ which git
/cygdrive/c/Program Files (x86)/Git/cmd/git
$ git config --list
core.symlinks=false
core.autocrlf=true
color.diff=auto
color.status=auto
color.branch=auto
color.interactive=true
pack.packsizelimit=2g
help.format=html
http.sslcainfo=/bin/curl-ca-bundle.crt
sendemail.smtpserver=/bin/msmtp.exe
diff.astextplain.textconv=astextplain
rebase.autosquash=true
user.name=Stefan
user.email=stefan@---.com
core.repositoryformatversion=0
core.filemode=false
core.bare=false
core.logallrefupdates=true
core.symlinks=false
core.ignorecase=true
core.hidedotfiles=dotGitOnly
-- Note core.autocrlf=true.
-- Created withcrlf.txt with one character and one CRLF line feed.
File size 3 bytes.
-- Created withlf.txt with one character and one LF line feed. File
size 2 bytes.
-- Now let's init repository.
$ git init
Initialized empty Git repository in D:/Dev/workspaces/gittest/.git/
$ ls -la
total 10
d---------+ 1 Stefan None 0 Jan 23 02:12 .
d---------+ 1 Stefan None 0 Jan 23 02:10 ..
d---------+ 1 Stefan None 0 Jan 23 02:13 .git
----------+ 1 Stefan None 3 Jan 23 01:55 withcrlf.txt
----------+ 1 Stefan None 2 Jan 23 01:55 withlf.txt
-- Note no .gitattributes file that will affect EOL conversion.
$ ls -la .git/info/
total 5
d---------+ 1 Stefan None 0 Jan 23 02:12 .
d---------+ 1 Stefan None 0 Jan 23 02:13 ..
----------+ 1 Stefan None 240 Jan 23 02:12 exclude
-- Note no attribute file in .git/info/ that will affect EOL conversion.
$ git add *
warning: LF will be replaced by CRLF in withlf.txt.
The file will have its original line endings in your working directory.
$ git commit -m 'Testing EOL'
[master (root-commit) 9a0b2f5] Testing EOL
2 files changed, 2 insertions(+)
create mode 100644 withcrlf.txt
create mode 100644 withlf.txt
warning: LF will be replaced by CRLF in withlf.txt.
The file will have its original line endings in your working directory.
$ ls -la
total 10
d---------+ 1 Stefan None 0 Jan 23 02:12 .
d---------+ 1 Stefan None 0 Jan 23 02:10 ..
d---------+ 1 Stefan None 0 Jan 23 02:22 .git
----------+ 1 Stefan None 3 Jan 23 01:55 withcrlf.txt
----------+ 1 Stefan None 2 Jan 23 01:55 withlf.txt
-- So no changes (see file size) to files in working directory. This
is expected and according to output warning from add and commit.
-- Now lets look in the repository
$ git ls-tree -l HEAD withcrlf.txt
100644 blob d00491fd7e5bb6fa28c517a0bb32b8b506539d4d 2 withcrlf.txt
$ git ls-tree -l HEAD withlf.txt
100644 blob d00491fd7e5bb6fa28c517a0bb32b8b506539d4d 2 withlf.txt
-- Note that size of withlf.txt is 2 in repository indicating that LF
was not replaced by CRLF in withlf.txt as indicated in output from add
and commit. Also note that size of withcrlf.txt is also 2 in
repository so it looks like CRLF was replaced by LF in withcrlf.txt.
To verify I will delete the files from working directory, turn off EOL
conversion, checkout files and look at file endings in the working
directory.
$ rm with*
$ ls -la
total 8
d---------+ 1 Stefan None 0 Jan 23 02:31 .
d---------+ 1 Stefan None 0 Jan 23 02:10 ..
d---------+ 1 Stefan None 0 Jan 23 02:22 .git
$ git status
# On branch master
# Changes not staged for commit:
# (use "git add/rm <file>..." to update what will be committed)
# (use "git checkout -- <file>..." to discard changes in working directory)
#
# deleted: withcrlf.txt
# deleted: withlf.txt
#
no changes added to commit (use "git add" and/or "git commit -a")
$ git config --global core.autocrlf false
$ git config --global core.autocrlf
false
$ git checkout -- with*
$ ls -la
total 10
d---------+ 1 Stefan None 0 Jan 23 02:38 .
d---------+ 1 Stefan None 0 Jan 23 02:10 ..
d---------+ 1 Stefan None 0 Jan 23 02:38 .git
----------+ 1 Stefan None 2 Jan 23 02:38 withcrlf.txt
----------+ 1 Stefan None 2 Jan 23 02:38 withlf.txt
-- Both files in working directory files now have LF line endings
confirming that files in repository have LF file endings. Either the
output message of add and commit is wrong or the behavior of the EOL
conversion is wrong... or... have I missed something...?
/Stefan
^ permalink raw reply
* Re: [PATCH v3 4/6] Change 'git' to 'Git' whenever the whole system is referred to #3
From: Junio C Hamano @ 2013-01-23 2:49 UTC (permalink / raw)
To: Thomas Ackermann; +Cc: davvid, git
In-Reply-To: <1667947659.632844.1358796098848.JavaMail.ngmail@webmail20.arcor-online.net>
Documentation/gitcore-tutorial.txt | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/gitcore-tutorial.txt b/Documentation/gitcore-tutorial.txt
index 5d957c2..59c1c17 100644
--- a/Documentation/gitcore-tutorial.txt
+++ b/Documentation/gitcore-tutorial.txt
@@ -1068,7 +1068,7 @@ and requires you to have a log-in privilege over `ssh` to the
remote machine. It finds out the set of objects the other side
lacks by exchanging the head commits both ends have and
transfers (close to) minimum set of objects. It is by far the
-most efficient way to exchange git objects between repositories.
+most efficient way to exchange Git objects between repositories.
Local directory::
`/path/to/repo.git/`
@@ -1077,7 +1077,7 @@ This transport is the same as SSH transport but uses 'sh' to run
both ends on the local machine instead of running other end on
the remote machine via 'ssh'.
-git Native::
+Git Native::
`git://remote.machine/path/to/repo.git/`
+
This transport was designed for anonymous downloading. Like SSH
--
1.8.1.1.507.g1754052
^ permalink raw reply related
* Re: [PATCH v3 3/6] Change 'git' to 'Git' whenever the whole system is referred to #2
From: Junio C Hamano @ 2013-01-23 2:48 UTC (permalink / raw)
To: Thomas Ackermann; +Cc: davvid, git
In-Reply-To: <7va9s0lnxn.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> As before, I'll locally create a fixup commit based on the above.
>
> After I am done with these patches, somebody (not Thomas or I, as
> our eyes are too used to staring at git and Git to notice mistakes)
> has to run "git grep -C3 -e '\<Git\>' Documentation/" and eyeball
> the output to spot mistakes that should have been "git" but was
> converted to "Git", as I am mostly concentrating on finding "git"
> that should have been converted in Thomas's series.
Documentation/config.txt | 4 ++--
Documentation/git-remote-helpers.txt | 2 +-
Documentation/git-verify-pack.txt | 2 +-
Documentation/gitattributes.txt | 4 ++--
4 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 5a831ad2..3652ee0 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2,7 +2,7 @@ CONFIGURATION FILE
------------------
The Git configuration file contains a number of variables that affect
-the git command's behavior. The `.git/config` file in each repository
+the Git commands' behavior. The `.git/config` file in each repository
is used to store the configuration for that repository, and
`$HOME/.gitconfig` is used to store a per-user configuration as
fallback values for the `.git/config` file. The file `/etc/gitconfig`
@@ -999,7 +999,7 @@ fetch.fsckObjects::
is used instead.
fetch.unpackLimit::
- If the number of objects fetched over the git native
+ If the number of objects fetched over the Git native
transfer is below this
limit, then the objects will be unpacked into loose object
files. However if the number of received objects equals or
diff --git a/Documentation/git-remote-helpers.txt b/Documentation/git-remote-helpers.txt
index e18c3b0..e36fdcb 100644
--- a/Documentation/git-remote-helpers.txt
+++ b/Documentation/git-remote-helpers.txt
@@ -39,7 +39,7 @@ INVOCATION
----------
Remote helper programs are invoked with one or (optionally) two
-arguments. The first argument specifies a remote repository as in git;
+arguments. The first argument specifies a remote repository as in Git;
it is either the name of a configured remote or a URL. The second
argument specifies a URL; it is usually of the form
'<transport>://<address>', but any arbitrary string is possible.
diff --git a/Documentation/git-verify-pack.txt b/Documentation/git-verify-pack.txt
index d4ae05b..0eb9ffb 100644
--- a/Documentation/git-verify-pack.txt
+++ b/Documentation/git-verify-pack.txt
@@ -3,7 +3,7 @@ git-verify-pack(1)
NAME
----
-git-verify-pack - Validate packed git archive files
+git-verify-pack - Validate packed Git archive files
SYNOPSIS
diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index b9c0eec..b322a26 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -449,7 +449,7 @@ String::
specify one or more options, as described in the following
section. The options for the diff driver "foo" are defined
by the configuration variables in the "diff.foo" section of the
- git config file.
+ Git config file.
Defining an external diff driver
@@ -675,7 +675,7 @@ attribute in the `.gitattributes` file:
*.ps -diff
------------------------
-This will cause git to generate `Binary files differ` (or a binary
+This will cause Git to generate `Binary files differ` (or a binary
patch, if binary patches are enabled) instead of a regular diff.
However, one may also want to specify other diff driver attributes. For
--
1.8.1.1.507.g1754052
^ permalink raw reply related
* Re: [PATCH v3 2/6] Change 'git' to 'Git' whenever the whole system is referred to #1
From: Junio C Hamano @ 2013-01-23 2:32 UTC (permalink / raw)
To: Thomas Ackermann; +Cc: davvid, git
In-Reply-To: <7vfw1slpig.fsf@alter.siamese.dyndns.org>
In Documentation/git-rev-parse.txt, there is this bit:
--resolve-git-dir <path>::
Check if <path> is a valid git-dir or a git-file pointing to
a valid git-dir. If <path> is a valid git-dir the resolved path to
git-dir will be printed.
I think the author invented the word "git-dir" to mean what we
ordinarily call $GIT_DIR (i.e. the directory that contains the
repository data), and used "git-file" to mean what the code and
error messages call "gitfile". I think it is better to leave these
in lowercase, but we would need them in the glossary, probably after
rewriting the latter to "gitfile". The former may want to be
rewritten to "gitdir" or even "$GIT_DIR".
^ permalink raw reply
* Re: [PATCH v3 0/2] Make git-svn work with gitdir links
From: Eric Wong @ 2013-01-23 2:32 UTC (permalink / raw)
To: Barry Wardell; +Cc: git, Junio C Hamano
In-Reply-To: <1358731322-44600-1-git-send-email-barry.wardell@gmail.com>
Barry Wardell <barry.wardell@gmail.com> wrote:
> These patches fix a bug which prevented git-svn from working with repositories
> which use gitdir links.
>
> Changes since v2:
> - Rebased onto latest master.
> - Added test case which verifies that the problem has been fixed.
> - Fixed problems with git svn (init|clone|multi-init).
> - All git-svn test cases now pass (except two in t9101 which also failed
> before these patches).
t9101 did not fail for me before your patches. However I have a
patch on top of your 2/2 which should fix things.
`git rev-parse --show-cdup` outputs nothing if GIT_DIR is set,
so I unset GIT_DIR temporarily.
I'm not sure why --show-cdup behaves like this, though..
Does squashing this on top of your changes fix all your failures?
I plan on squashing both your changes together with the below:
diff --git a/git-svn.perl b/git-svn.perl
index c232798..e5bd292 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -332,11 +332,13 @@ if ($cmd && $cmd =~ /(?:clone|init|multi-init)$/) {
$ENV{GIT_DIR} = command_oneline([qw/rev-parse --git-dir/]);
} "Unable to find .git directory\n";
my $cdup = undef;
+ my $git_dir = delete $ENV{GIT_DIR};
git_cmd_try {
$cdup = command_oneline(qw/rev-parse --show-cdup/);
chomp $cdup if ($cdup);
$cdup = "." unless ($cdup && length $cdup);
- } "Already at toplevel, but $ENV{GIT_DIR} not found\n";
+ } "Already at toplevel, but $git_dir not found\n";
+ $ENV{GIT_DIR} = $git_dir;
chdir $cdup or die "Unable to chdir up to '$cdup'\n";
$_repository = Git->repository(Repository => $ENV{GIT_DIR});
}
^ permalink raw reply related
* Re: [PATCH v3 2/6] Change 'git' to 'Git' whenever the whole system is referred to #1
From: Junio C Hamano @ 2013-01-23 2:24 UTC (permalink / raw)
To: Thomas Ackermann; +Cc: davvid, git
In-Reply-To: <7vfw1slpig.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> This is the summary of the review comments in a patch form.
>
And then there still are the ones in the file that was touched by
Thomas's 2/6 that need fixing.
Documentation/git-apply.txt | 2 +-
Documentation/git-archimport.txt | 2 +-
Documentation/git-cvsserver.txt | 8 ++++----
Documentation/git-help.txt | 2 +-
Documentation/git-mergetool--lib.txt | 2 +-
Documentation/git-p4.txt | 4 ++--
6 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt
index c1704dc..f605327 100644
--- a/Documentation/git-apply.txt
+++ b/Documentation/git-apply.txt
@@ -198,7 +198,7 @@ behavior:
* `fix` outputs warnings for a few such errors, and applies the
patch after fixing them (`strip` is a synonym --- the tool
used to consider only trailing whitespace characters as errors, and the
- fix involved 'stripping' them, but modern gits do more).
+ fix involved 'stripping' them, but modern Gits do more).
* `error` outputs warnings for a few such errors, and refuses
to apply the patch.
* `error-all` is similar to `error` but shows all errors.
diff --git a/Documentation/git-archimport.txt b/Documentation/git-archimport.txt
index 25b1cab..163b9f6 100644
--- a/Documentation/git-archimport.txt
+++ b/Documentation/git-archimport.txt
@@ -3,7 +3,7 @@ git-archimport(1)
NAME
----
-git-archimport - Import an Arch repository into git
+git-archimport - Import an Arch repository into Git
SYNOPSIS
diff --git a/Documentation/git-cvsserver.txt b/Documentation/git-cvsserver.txt
index dc5069f..7ca64b3 100644
--- a/Documentation/git-cvsserver.txt
+++ b/Documentation/git-cvsserver.txt
@@ -3,7 +3,7 @@ git-cvsserver(1)
NAME
----
-git-cvsserver - A CVS server emulator for git
+git-cvsserver - A CVS server emulator for Git
SYNOPSIS
--------
@@ -291,14 +291,14 @@ Variable substitution
In `dbdriver` and `dbuser` you can use the following variables:
%G::
- git directory name
+ Git directory name
%g::
- git directory name, where all characters except for
+ Git directory name, where all characters except for
alpha-numeric ones, `.`, and `-` are replaced with
`_` (this should make it easier to use the directory
name in a filename if wanted)
%m::
- CVS module/git head name
+ CVS module/Git head name
%a::
access method (one of "ext" or "pserver")
%u::
diff --git a/Documentation/git-help.txt b/Documentation/git-help.txt
index 835fba0..e07b6dc 100644
--- a/Documentation/git-help.txt
+++ b/Documentation/git-help.txt
@@ -3,7 +3,7 @@ git-help(1)
NAME
----
-git-help - display help information about git
+git-help - Display help information about Git
SYNOPSIS
--------
diff --git a/Documentation/git-mergetool--lib.txt b/Documentation/git-mergetool--lib.txt
index b748bdf..055550b 100644
--- a/Documentation/git-mergetool--lib.txt
+++ b/Documentation/git-mergetool--lib.txt
@@ -3,7 +3,7 @@ git-mergetool{litdd}lib(1)
NAME
----
-git-mergetool--lib - Common git merge tool shell scriptlets
+git-mergetool--lib - Common Git merge tool shell scriptlets
SYNOPSIS
--------
diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index 96a5b31..1d1b020 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -64,7 +64,7 @@ COMMANDS
Clone
~~~~~
-Generally, 'git p4 clone' is used to create a new git directory
+Generally, 'git p4 clone' is used to create a new Git directory
from an existing p4 repository:
------------
$ git p4 clone //depot/path/project
@@ -494,7 +494,7 @@ git-p4.detectCopiesHarder::
Detect copies harder. See linkgit:git-diff[1]. A boolean.
git-p4.preserveUser::
- On submit, re-author changes to reflect the git author,
+ On submit, re-author changes to reflect the Git author,
regardless of who invokes 'git p4 submit'.
git-p4.allowMissingP4Users::
^ 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