* 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 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 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
* 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 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
* 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 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 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
* [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
* [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 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 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 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 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 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 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 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
* 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
* 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] 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 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: [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: [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: [PATCH] all: new command used for multi-repo operations
From: Lars Hjemli @ 2013-01-23 7:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: David Aguilar, git
In-Reply-To: <7vy5fkiek0.fsf@alter.siamese.dyndns.org>
On Wed, Jan 23, 2013 at 7:44 AM, Junio C Hamano <gitster@pobox.com> wrote:
> 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?
Yes, this is needed to meet one of the goals for git-all: to be an
alternative to git-submodule when submodules doesn't fit (the other
goal is to manage a collection of unrelated (not nested) repos).
--
larsh
^ permalink raw reply
* [PATCH v2] all: new command used for multi-repo operations
From: Lars Hjemli @ 2013-01-23 8:12 UTC (permalink / raw)
To: git; +Cc: Lars Hjemli
When working with multiple, unrelated (or loosly related) git repos,
there is often a need to locate all repos with uncommitted work and
perform some action on them (say, commit and push). Before this patch,
such tasks would require manually visiting all repositories, running
`git status` within each one and then decide what to do next.
This mundane task can now be automated by e.g. `git all --dirty status`,
which will find all git repositories below the current directory (even
nested ones), check if they are dirty (as defined by `git diff --quiet &&
git diff --cached --quiet`), and for each dirty repo print the path to the
repo and then execute `git status` within the repo.
The command also honours the option '--clean' which restricts the set of
repos to those which '--dirty' would skip.
Finally, the command to execute within each repo is optional. If none is
given, git-all will just print the path to each repo found.
Signed-off-by: Lars Hjemli <hjemli@gmail.com>
---
Changes since v1:
* uses setenv() instead of chdir(), which fixes .gitfile handling
* uses DTYPE() and stat(), which fixes NO_D_TYPE_IN_DIRENT platforms
* uses OPT_SET_INT() instead of OPT_BOOLEAN
* support for --all (complements --clean/--dirty)
* removed from command-list.txt
* added to .gitignore
I've not yet renamed the command. If it should be changed to 'git
for-each-repo', I'm tempted to make a patch which transforms
`git -ad status` into `git for-each-repo -d status`.
.gitignore | 1 +
Documentation/git-all.txt | 42 ++++++++++++++++++
Makefile | 1 +
builtin.h | 1 +
builtin/all.c | 110 ++++++++++++++++++++++++++++++++++++++++++++++
git.c | 1 +
t/t0064-all.sh | 46 +++++++++++++++++++
7 files changed, 202 insertions(+)
create mode 100644 Documentation/git-all.txt
create mode 100644 builtin/all.c
create mode 100755 t/t0064-all.sh
diff --git a/.gitignore b/.gitignore
index aa258a6..27118d7 100644
--- a/.gitignore
+++ b/.gitignore
@@ -10,6 +10,7 @@
/git
/git-add
/git-add--interactive
+/git-all
/git-am
/git-annotate
/git-apply
diff --git a/Documentation/git-all.txt b/Documentation/git-all.txt
new file mode 100644
index 0000000..baaa57e
--- /dev/null
+++ b/Documentation/git-all.txt
@@ -0,0 +1,42 @@
+git-all(1)
+==========
+
+NAME
+----
+git-all - Execute a git command in multiple repositories
+
+SYNOPSIS
+--------
+[verse]
+'git all' [--all|--clean|--dirty] [command]
+
+DESCRIPTION
+-----------
+The git-all command is used to locate all git repositoris within the
+current directory tree, and optionally execute a git command in each
+of the found repos.
+
+OPTIONS
+-------
+-a::
+--all::
+ Include both clean and dirty repositories (this is the default
+ behaviour of `git-all`).
+
+-c::
+--clean::
+ Only include repositories with a clean worktree.
+
+-d::
+--dirty::
+ Only include repositories with a dirty worktree.
+
+NOTES
+-----
+
+For the purpose of `git-all`, a dirty worktree is defined as a worktree
+with uncommitted changes.
+
+GIT
+---
+Part of the linkgit:git[1] suite
diff --git a/Makefile b/Makefile
index 1b30d7b..8bf0583 100644
--- a/Makefile
+++ b/Makefile
@@ -840,6 +840,7 @@ LIB_OBJS += xdiff-interface.o
LIB_OBJS += zlib.o
BUILTIN_OBJS += builtin/add.o
+BUILTIN_OBJS += builtin/all.o
BUILTIN_OBJS += builtin/annotate.o
BUILTIN_OBJS += builtin/apply.o
BUILTIN_OBJS += builtin/archive.o
diff --git a/builtin.h b/builtin.h
index 7e7bbd6..438c265 100644
--- a/builtin.h
+++ b/builtin.h
@@ -41,6 +41,7 @@ void finish_copy_notes_for_rewrite(struct notes_rewrite_cfg *c);
extern int textconv_object(const char *path, unsigned mode, const unsigned char *sha1, int sha1_valid, char **buf, unsigned long *buf_size);
extern int cmd_add(int argc, const char **argv, const char *prefix);
+extern int cmd_all(int argc, const char **argv, const char *prefix);
extern int cmd_annotate(int argc, const char **argv, const char *prefix);
extern int cmd_apply(int argc, const char **argv, const char *prefix);
extern int cmd_archive(int argc, const char **argv, const char *prefix);
diff --git a/builtin/all.c b/builtin/all.c
new file mode 100644
index 0000000..b170b26
--- /dev/null
+++ b/builtin/all.c
@@ -0,0 +1,110 @@
+/*
+ * "git all" builtin command.
+ *
+ * Copyright (c) 2013 Lars Hjemli <hjemli@gmail.com>
+ */
+#include "cache.h"
+#include "color.h"
+#include "builtin.h"
+#include "run-command.h"
+#include "parse-options.h"
+
+#define ALL 0
+#define DIRTY 1
+#define CLEAN 2
+
+static int match;
+
+static const char * const builtin_all_usage[] = {
+ N_("git all [--all|--clean|--dirty] [cmd]"),
+ NULL
+};
+
+static struct option builtin_all_options[] = {
+ OPT_SET_INT('a', "all", &match, N_("match both clean and dirty repositories"), ALL),
+ OPT_SET_INT('c', "clean", &match, N_("only show clean repositories"), CLEAN),
+ OPT_SET_INT('d', "dirty", &match, N_("only show dirty repositories"), DIRTY),
+ OPT_END(),
+};
+
+static int get_repo_state()
+{
+ const char *diffidx[] = {"diff", "--quiet", "--cached", NULL};
+ const char *diffwd[] = {"diff", "--quiet", NULL};
+
+ if (run_command_v_opt(diffidx, RUN_GIT_CMD) != 0)
+ return DIRTY;
+ if (run_command_v_opt(diffwd, RUN_GIT_CMD) != 0)
+ return DIRTY;
+ return CLEAN;
+}
+
+static void handle_repo(char *path, const char **argv)
+{
+ if (path[0] == '.' && path[1] == '/')
+ path += 2;
+ if (match != ALL && match != get_repo_state())
+ return;
+ if (*argv) {
+ color_fprintf_ln(stdout, GIT_COLOR_YELLOW, "[%s]", path);
+ run_command_v_opt(argv, RUN_GIT_CMD);
+ } else
+ printf("%s\n", path);
+}
+
+static int walk(struct strbuf *path, int argc, const char **argv)
+{
+ DIR *dir;
+ struct dirent *ent;
+ struct stat st;
+ 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_addstr(path, ent->d_name);
+ setenv(GIT_DIR_ENVIRONMENT, path->buf, 1);
+ strbuf_setlen(path, len - 1);
+ setenv(GIT_WORK_TREE_ENVIRONMENT, path->buf, 1);
+ handle_repo(path->buf, argv);
+ strbuf_addstr(path, "/");
+ continue;
+ }
+ strbuf_setlen(path, len);
+ strbuf_addstr(path, ent->d_name);
+ switch (DTYPE(ent)) {
+ case DT_UNKNOWN:
+ /* Use stat() instead of lstat(), since we want to
+ * know if we can follow this path into another
+ * directory - it's not important if it's actually
+ * a symlink which gets us there.
+ */
+ if (stat(path->buf, &st) || !S_ISDIR(st.st_mode))
+ break;
+ /* fallthrough */
+ case DT_DIR:
+ walk(path, argc, argv);
+ break;
+ }
+ strbuf_setlen(path, len);
+ }
+ closedir(dir);
+ return 0;
+}
+
+int cmd_all(int argc, const char **argv, const char *prefix)
+{
+ struct strbuf path = STRBUF_INIT;
+
+ argc = parse_options(argc, argv, prefix, builtin_all_options,
+ builtin_all_usage, PARSE_OPT_STOP_AT_NON_OPTION);
+
+ strbuf_addstr(&path, ".");
+ return walk(&path, argc, argv);
+}
diff --git a/git.c b/git.c
index ed66c66..53fd963 100644
--- a/git.c
+++ b/git.c
@@ -304,6 +304,7 @@ static void handle_internal_command(int argc, const char **argv)
const char *cmd = argv[0];
static struct cmd_struct commands[] = {
{ "add", cmd_add, RUN_SETUP | NEED_WORK_TREE },
+ { "all", cmd_all },
{ "annotate", cmd_annotate, RUN_SETUP },
{ "apply", cmd_apply, RUN_SETUP_GENTLY },
{ "archive", cmd_archive },
diff --git a/t/t0064-all.sh b/t/t0064-all.sh
new file mode 100755
index 0000000..3738ab2
--- /dev/null
+++ b/t/t0064-all.sh
@@ -0,0 +1,46 @@
+#!/bin/sh
+#
+# Copyright (c) 2013 Lars Hjemli
+#
+
+test_description='Test the git-all command'
+
+. ./test-lib.sh
+
+test_expect_success "setup" '
+ test_create_repo clean &&
+ (cd clean && test_commit foo) &&
+ git init --separate-git-dir=.cleansub clean/gitfile &&
+ (cd clean/gitfile && test_commit foo && echo bar >>foo.t) &&
+ test_create_repo dirty-wt &&
+ (cd dirty-wt && test_commit foo && rm foo.t) &&
+ test_create_repo dirty-idx &&
+ (cd dirty-idx && test_commit foo && git rm foo.t)
+'
+
+test_expect_success "without flags, all repos are included" '
+ echo "." >expect &&
+ echo "clean" >>expect &&
+ echo "clean/gitfile" >>expect &&
+ echo "dirty-idx" >>expect &&
+ echo "dirty-wt" >>expect &&
+ git all | sort >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success "--dirty only includes dirty repos" '
+ echo "clean/gitfile" >expect &&
+ echo "dirty-idx" >>expect &&
+ echo "dirty-wt" >>expect &&
+ git all --dirty | sort >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success "--clean only includes clean repos" '
+ echo "." >expect &&
+ echo "clean" >>expect &&
+ git all --clean | sort >actual &&
+ test_cmp expect actual
+'
+
+test_done
--
1.8.1.1.296.g725455c
^ 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